On 24.05.2019 18:27, Josh Allmann wrote:
On Fri, 24 May 2019 at 06:00, Timo Rothenpieler <t...@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#l380It 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.
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
