Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] player/command: reinit video chain when decoder thread count changes #14119

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christoph-heinrich
Copy link
Contributor

My goal is to reinitialize the video decoder when the vd-lavc-threads option changes.
This reinitializes the whole video chain, no idea how this could be restricted to the decoder only without touching the rest of the chain or if that's even desirable.

I don't really know what I'm doing here, so there is probably a much better way to achieve this.

We could also extend this to other decoder related options once the approach has been figured out.


#include <stdbool.h>

struct vd_lavc_params {
Copy link
Contributor

@kasper93 kasper93 May 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hidden as private for a reason. You can add a function that acts if any of the setting has changed. And check in command.c opt_ptr == &opts->vd_lavc_params only. EDIT: this will not work, but something like that has to be done. (just implement the check in vd_lavc.c

This is hidden to avoid leaking implementation details over other modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So instead of the struct the header defines a function that takes the pointer and context and then does the if from command.c in there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function also doesn't fit into the design looking at it.

If fact common way is to use flag like flags & UPDATE_VD and reinit then. Mark option with this flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's much better, thanks.

It looks to me that we could rename UPDATE_HWDEC to UPDATE_VD and do the same thing for any of those options instead of only the hwdec one.
Doing that required changing the absolute seek to a relative one, otherwise it wouldn't work well with vd-queue (starting a video would offset the starting position by vd-queue-secs seconds).

I've pushed that change and it seems to work fine for hw and sw decoding, but I didn't try all related options.

Copy link

github-actions bot commented May 11, 2024

Download the artifacts for this pull request:

Windows
macOS

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented May 11, 2024

Using this with a conditional auto profile

[auto-vp9-vd-threads]
profile-cond=get('current-tracks/video/codec') == 'vp9'
profile-restore=copy
vd-lavc-threads=12

leads to those errors repeated a bunch of times for local files (not for networks streams)

[ffmpeg/video] vp9: Not all references are available
Error while decoding frame!

Everything works fine, but that's not great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants