[FFmpeg-devel] [PATCH] lavfi/vf_scale_cuda: Add format option

[Timo Rothenpieler]

On 24.05.2019 18:27, Josh Allmann wrote:
> On Fri, 24 May 2019 at 06:00, Timo Rothenpieler <timo at rothenpieler.org> wrote:
>>
>> On 24/05/2019 01:49, Josh Allmann wrote:
>>> Makes certain usages of the lavfi API easier.
>>> ---
>>>    libavfilter/vf_scale_cuda.c | 12 +++++++++++-
>>>    1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavfilter/vf_scale_cuda.c b/libavfilter/vf_scale_cuda.c
>>> index b7cdb81081..6b1ef2bb6f 100644
>>> --- a/libavfilter/vf_scale_cuda.c
>>> +++ b/libavfilter/vf_scale_cuda.c
>>> @@ -81,6 +81,7 @@ typedef struct CUDAScaleContext {
>>>
>>>        char *w_expr;               ///< width  expression string
>>>        char *h_expr;               ///< height expression string
>>> +    char *format_str;
>>>
>>>        CUcontext   cu_ctx;
>>>        CUmodule    cu_module;
>>> @@ -101,7 +102,15 @@ static av_cold int cudascale_init(AVFilterContext *ctx)
>>>    {
>>>        CUDAScaleContext *s = ctx->priv;
>>>
>>> -    s->format = AV_PIX_FMT_NONE;
>>> +    if (!strcmp(s->format_str, "same")) {
>>> +        s->format = AV_PIX_FMT_NONE;
>>> +    } else {
>>> +        s->format = av_get_pix_fmt(s->format_str);
>>> +        if (s->format == AV_PIX_FMT_NONE) {
>>> +            av_log(ctx, AV_LOG_ERROR, "Unrecognized pixel format: %s\n", s->format_str);
>>> +            return AVERROR(EINVAL);
>>> +        }
>>> +    }
>>>        s->frame = av_frame_alloc();
>>>        if (!s->frame)
>>>            return AVERROR(ENOMEM);
>>> @@ -533,6 +542,7 @@ fail:
>>>    static const AVOption options[] = {
>>>        { "w",      "Output video width",  OFFSET(w_expr),     AV_OPT_TYPE_STRING, { .str = "iw"   }, .flags = FLAGS },
>>>        { "h",      "Output video height", OFFSET(h_expr),     AV_OPT_TYPE_STRING, { .str = "ih"   }, .flags = FLAGS },
>>> +    { "format", "Output pixel format", OFFSET(format_str), AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
>>>        { NULL },
>>>    };
>>
>> I'm not sure what to think about a dummy option like this. It might be
>> very confusing for users to see a format option, which only accepts a
>> single value, "same", and effectively does nothing.
>>
> 
> Not sure I understand the issue.  "same" is the default (terminology
> borrowed from the scale_npp filter), and it'll assign the format to
> whatever is passed in (eg, format=yuv420p assigns that).

Oh, I misread that code as just always throwing an error if it's != "same".

Unfortunately, that option is omitted for a reason.
If you look at scalecuda_resize:
https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavfilter/vf_scale_cuda.c;h=b7cdb81081ff4a34e7b641c533fc23a5714fed61;hb=HEAD#l380

It has the assumption built into it that the output frame has the same 
format as the input frame. So if you were to set format=nv12 and then 
input a yuv420p frame, this will most likely crash or at least severely 
misbehave.

I would not be opposed to scale_cuda gaining the ability to also change 
frame pix_fmts, we are lacking such a filter at the moment if one 
ignores scale_npp.
But in its current state, it can't do that.

>>
>> Not strictly against it, since I can see the convenience it adds when
>> building command lines, but I'd like some second opinions on this.
>>
> 
> Actually I'm using the API, albeit with some of lavfi conveniences to
> parse filter strings. This avoids "wiring in" the output format
> manually when crossing the lavfi boundary.
> 
> Here's a example that demonstrates the issue via CLI (this may
> actually be a bug elsewhere?):
> 
> Broken:
> ffmpeg -loglevel verbose -hwaccel cuvid -c:v h264_cuvid -i input.ts
> -an -lavfi scale_cuda=w=426:h=240,hwdownload,format=yuv420p -c:v
> libx264 out.ts
> 
> Working:
> ffmpeg -loglevel verbose -hwaccel cuvid -c:v h264_cuvid -i input.ts
> -an -lavfi scale_cuda=w=426:h=240:format=yuv420p,hwdownload,format=yuv420p
> -c:v libx264 out.ts

Is this actually working in a sense where the result looks sensible?
Cause with how the code currently is, scale_cuda with format set to 
yuv420p and getting nv12 as input from h264_cuvid will produce a frame 
labeled as yuv420p but containing nv12 data.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4538 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190524/92a92fdd/attachment.bin>