On 25/06/17 00:40, Philip Langdale wrote:
> Second try.
>
> Feedback on first proposal was lack of interest in having default behaviour
> vary between hwaccels, despite their other differences.
>
> In this patch series, I instead force the user to confront the change in
> command line semantics when doing cuvid/nvenc transcoding. They will only
> get full transcoding with an additional command line argument, so force
> them to specify that argument.
>
> Philip Langdale (2):
> ffmpeg: Switch cuvid to generic hwaccel
> ffmpeg: Require output format when using cuvid hwaccel
>
> Makefile | 1 -
> ffmpeg.h | 2 +-
> ffmpeg_cuvid.c | 73
> ----------------------------------------------------------
> ffmpeg_opt.c | 31 ++++++++++++++++++-------
> 4 files changed, 24 insertions(+), 83 deletions(-)
> delete mode 100644 ffmpeg_cuvid.c
Can we take a step back here and consider what we actually want the result to
be?
I think what we want to achieve is:
1) CUDA devices passed to CUVID decoders as AVCodecContext.hw_device_ctx:
a) If we have exactly one device, always pass it.
b) If we have multiple devices, allow -hwaccel_device to select which one to
pass (fail or make an arbitrary choice if not present?).
c) If none, either make a default one or pass nothing (depends what works).
* Note that this decision logic is completely unaffected by whether the dummy
hwaccel is specified or not.)
2) Output / get_format
a) If the dummy hwaccel is specified, output in hwaccel_output_format or
default to CUDA frames if not specified.
b) Otherwise, always output in normal memory.
* Alternatively, actually change the semantics and be clear about what the new
ones are.
3) Remove all of the CUVID-specific code from ffmpeg.
Point (1) pretty much already works without any change - see
hw_device_setup_for_decode(). The only missing feature here is that the name
matching doesn't actually work for CUVID (strstr("h264_cuvid", "cuda") ->
NULL). I'm not sure that's actually a problem, though - the CUVID decoder is
happy to pull a new device instance out of nothing, so it isn't actually needed
in the standalone device case. A hack there could give slightly more
consistent behaviour, though.
(An imagined solution to avoid the string matching completely was to add a
field to AVCodec which contains the set of devices that it can use, but that's
a more invasive library change that noone has pursued yet.)
Point (2) is somewhat more subtle. The default hwaccel behaviour is made for
the real hwaccels attached to the normal decoder, and won't do the right thing
for the dummy ones. The specific case that we strongly want to avoid is some
normal setting where the output is downloaded to normal memory from a hardware
frame inside ffmpeg, because that is almost certainly done more efficiently in
the decoder itself (for the CUVID case, it actually has two explicit copies if
you do this). It's rare that you ever want to specify anything other than the
hardware format or the corresponding software format for decode (and in fact I
think only VAAPI supports such convert-on-download cases anyway), so the single
toggle is usually sufficient.
Therefore, I think we should just clearly distinguish between the two general
behaviours for the hwaccel case - real and dummy. That's essentially what your
patch at
<https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-June/212566.html> did,
but in a slightly implicit way - I would put it in hwaccel_decode_init() rather
than in the option parsing code.
There was some question of whether all hwaccels (real and dummy) should behave
identically here, but given the nasty default case if we do that I don't think
it's justified (though feel free to argue further on this point if you feel
strongly, I'm not that attached to it).
TL;DR - I preferred the mechanism of the previous version, with some changes to
make it clearer what the distinction is.
Thanks,
- Mark
PS: The libmfx behaviour unfortunately isn't really a very useful comparison,
because the implementation is incomplete. The important change with the
generic code was that the string matching does the right thing to create a
device for the non-hwaccel case. The hwaccel case didn't change at all because
hw_device_ctx + hardware frame output isn't supported in the decoder. I don't
have any plans to implement it soon because I regard the libmfx decoders as
mostly deprecated at this point (use real hwaccel for the platform + hwmap for
encode instead), but I wouldn't mind it being done by someone else if they care.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
