Opened 5 years ago

Last modified 5 years ago

#7564 new defect

hwdownload fails when size changes

Reported by: Mark Sanders Owned by:
Priority: normal Component: avfilter
Version: git-master Keywords: hwdownload
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:

When some filter changes the size of the frames, then the "hwdownload" filter doesn't produce the correct output.

How to reproduce, Test 1 working:

% ffmpeg -stream_loop -1 -y \
 -loglevel error \
 -hwaccel cuvid \
 -c:v h264_cuvid \
   -i "../Day Flight.mpg" \
 -filter_complex "hwdownload,format=nv12,scale=400:300,hwupload,scale_cuda=800:600,hwdownload,format=nv12,fps=25,scale=800:600" \
 -c:v h264 \
 -f mpegts out.ts

When running this play, pause the execution with key <c> and type

Parsed_scale_2 -1 w 200

This changes the behaviour of the first software scale filter to a width of 200.
And the result is correct.

How to reproduce, Test 2 with errors:

% ffmpeg -stream_loop -1 -y \
 -loglevel error \
 -hwaccel cuvid \
 -c:v h264_cuvid \
   -i "../Day Flight.mpg" \
 -filter_complex "hwdownload,format=nv12,scale=400:300,hwupload,hwdownload,format=nv12,fps=25,scale=800:600" \
 -c:v h264 \
 -f mpegts out.ts

Then repeat the change of the software scaling filter with "Parsed_scale_2 -1 w 200".
Now the output has a vertical green line above pixels 200, and the image only occupies the left side. The right side is occupied by ancient still image.

So, the problem is: the "hwupload" filter does the correct behaviour. However, the "hwdownload" filter doesn't.

Please, fix this bug!

Attachments (2)

before.png (459.0 KB ) - added by Mark Sanders 5 years ago.
output after the command, all OK
after.png (467.7 KB ) - added by Mark Sanders 5 years ago.
capture after the command, showing the BUG

Download all attachments as: .zip

Change History (13)

comment:1 by Carl Eugen Hoyos, 5 years ago

Component: avfilterundetermined
Keywords: cuda added; hwdownload resize removed

Please provide the command line you tested together with the complete, uncut console output to make this a valid ticket.

in reply to:  1 comment:2 by Mark Sanders, 5 years ago

Replying to cehoyos:

Please provide the command line you tested together with the complete, uncut console output to make this a valid ticket.

Hi,

No error is printed in the log (please, set that "-loglevel error" is used).
This is because the error is found only in the output file!

The input file can be any file. However, I use the "Day Flight.mpg" shared as example in the ffmpeg repository: https://samples.ffmpeg.org/MPEG2/mpegts-klv/Day%20Flight.mpg

Futhermore, you can check in realtime the result sending the output to the network and playing it:

% ffmpeg -stream_loop -1 -y \
 -loglevel error \
 -hwaccel cuvid \
 -c:v h264_cuvid \
   -i "https://samples.ffmpeg.org/MPEG2/mpegts-klv/Day%20Flight.mpg" \
 -filter_complex "hwdownload,format=nv12,scale=400:300,hwupload,hwdownload,format=nv12,fps=25,scale=800:600" \
 -c:v h264 \
 -f mpegts -muxrate 6M "udp://@239.0.0.1:123456?pkt_size=1316"

And then play it in another shell with

% ffplay udp://@239.0.0.1:123456

And execute in the the first at any time

<c> Parsed_scale_2 -1 w 200

Finally, I need to comment that even I use the CUDA hardware, the problem isn't related to CUDA. The problem is related to the "hwdownload" filter. In my example I use the CUDA hwaccel as it's the one available to me. You can use others, and the result should be the same. Check it!

The problem is because the hwdownload doesn't catch size changes!

I put two examples (the first with a "scale_cuda" filter) to demonstrate that the "hwupload" works as expected. As when changing the size, the "scale_cuda" accepts the new size. The problem appears when using "...,scale=400:300,hwupload,hwdownload,...", and the "scale" filter changes the resolution.

This is a bug!

comment:3 by Mark Sanders, 5 years ago

Component: undeterminedavfilter
Keywords: hwdownload added; cuda removed

comment:4 by Timo R., 5 years ago

Hwdownload shouldn't need to catch anything, as you are changing the settings on the last scale filter in the chain, so nothing changes for any of the hw related filters, or am I missing something?

in reply to:  4 comment:5 by Mark Sanders, 5 years ago

Replying to oromit:

Hwdownload shouldn't need to catch anything, as you are changing the settings on the last scale filter in the chain, so nothing changes for any of the hw related filters, or am I missing something?

Hi oromit,

With all due respect... yes you missed all! ;-)

Parsed_scale_2 is the first "scale" filter in the graph.

This is what's happening:

  • The hw decoder (used only to enable the hwaccel) is decoding.
  • We do a hwdownload to move to RAM.
  • We do a software scale (aka Parsed_scale_2).
  • We upload to GPU.
  • We download from GPU.
  • Finally, only to maintain the resolution in the final output, we do a second reescale and encode.

And all works as expected. As all filters catch the change in the size... except for the "hwdownload". Why I know it?

  • Because if we interpose another rescale (in this case in gpu, with "scale_cuda") in between the "hwupload" and the "hwdownload" then the output is correct. This happens because the "hwupload" catch the size change, and as the "hwdownload" doesn't see any size change, then nothing fails.

Please, don't focuse in this "example". I created it to demonstrate the bug! I see the problem when using a GPU reescaler chaging the size on-the-fly. It's an unobserved bug so far.

To be more clear, I attach to images. The "before.png" is the output before the command. The second "after.png" is when the error appears after executing the command "Parsed_scale_2 -1 w 200".

Regards.

by Mark Sanders, 5 years ago

Attachment: before.png added

output after the command, all OK

by Mark Sanders, 5 years ago

Attachment: after.png added

capture after the command, showing the BUG

comment:6 by Mark Sanders, 5 years ago

Sorry! File names are interchanged. Please, note it!

comment:7 by Mark Sanders, 5 years ago

Hi,

And to demonstrate that the bug isn't related to CUDA, but to "hwdownload" I ported the example to QSV:

% ffmpeg -stream_loop -1 -y \
 -loglevel error \
 -hwaccel qsv \
 -c:v h264_qsv \
   -i "https://samples.ffmpeg.org/MPEG2/mpegts-klv/Day%20Flight.mpg" \
 -filter_complex "hwdownload,format=nv12,scale=400:300,hwupload=extra_hw_frames=32,hwdownload,format=nv12,fps=25,scale=800:600" \
 -c:v h264 \
 -f mpegts out.ts

The output shows the exact same bug!

comment:8 by Mark Sanders, 5 years ago

Hi,

Peharps the problem is related to the fact that the "hwdownload" filter is using the size from the output:

https://github.com/FFmpeg/FFmpeg/blob/90ac0e5f29ba4730cd92d3268938b3730823e52b/libavfilter/vf_hwdownload.c#L158

    output->width  = outlink->w;
    output->height = outlink->h;

As the "hwupload" gets it from the input:
https://github.com/FFmpeg/FFmpeg/blob/90ac0e5f29ba4730cd92d3268938b3730823e52b/libavfilter/vf_hwupload.c#L172

    output->width  = input->width;
    output->height = input->height;

comment:9 by Timo R., 5 years ago

To avoid such confusion, please provide the full uncut output like asked at the very beginning in the future.

The outlink w/h parameters are set during config_props, where they are received from the hw_frames_ctx, which is created and set in the hwupload filter.
Unfortunately that means in its current state both hwupload and hwdownload do not support dynamic size changes and cannot be easily changed to do so.

hwupload is the much bigger issue here. It will keep using the original hw_frames_ctx, which after the size change either allocates too big(waste of space) of a buffer or even too small of a buffer(which probably results in a crash).

Now it could be changed to monitor for input parameter changes, and then re-create a new hw_frames_ctx. But all downstream filters are already configured to the old context, and do not expect it to change at runtime.

Support for runtime parameter changes is also very spotty with pure sw filtering. With a hw filter chain it would be a major undertaking affecting every single hw filter.

So the conclusion here is, that without a lot of bigger changes, runtime parameter changes like this are unsupported with a filter chain that uses hardware frames.

in reply to:  9 comment:10 by Mark Sanders, 5 years ago

Replying to oromit:

To avoid such confusion, please provide the full uncut output like asked at the very beginning in the future.

Here it is:

Please, note the "-loglevel error" in the example.
And this isn't a joke.

.

The outlink w/h parameters are set during config_props, where they are received from the hw_frames_ctx, which is created and set in the hwupload filter.

These values are set correctly at the beginning.
Nothing to comment about it.

.

Unfortunately that means in its current state both hwupload and hwdownload do not support dynamic size changes and cannot be easily changed to do so.

Bravo! You got it!

But, the problem is not that "they do not support dynamic size changes". All video filters at time have such support. And in fact, "hwupload" (as I described and demonstrated) doesn't have any problem. That's it supports it.

The problem is that "hwdownload" doesn't do it. And this is a bug!

.

hwupload is the much bigger issue here. It will keep using the original hw_frames_ctx, which after the size change either allocates too big(waste of space) of a buffer or even too small of a buffer(which probably results in a crash).

Now we're beginning to understand each other.
However, the "hwupload" doesn't have any problem. Please, see the first test with a "scale_cuda" interposed. It doens't fail at all!

But, Yes! When changing the size of frames, the HW Buffer has troubles as it is not resizable (at time). However, at least the size should be able to be reduced without changing the allocated space. This is true for other HW filters. For example "scale_cuda" doesn't have any problem when the software scale resizes the input to a lower value. It handles it correctly.

.

Now it could be changed to monitor for input parameter changes, and then re-create a new hw_frames_ctx. But all downstream filters are already configured to the old context, and do not expect it to change at runtime.

Yes. I see this problem. The hw context can't be changed without an important rewrite of the current code.
We can live with this limitation until a huge rewrite could be done.

.

Support for runtime parameter changes is also very spotty with pure sw filtering. With a hw filter chain it would be a major undertaking affecting every single hw filter.

So the conclusion here is, that without a lot of bigger changes, runtime parameter changes like this are unsupported with a filter chain that uses hardware frames.

Sorry. I need to repeat it: ONLY the "hwdownload" filter has problems reducing the size. I check it. And as I pointed the problem seems to be related to uncorrect code.

If not, then try to explain why interposing the filter "scale_cuda" and reducing the size all goes right.

comment:11 by Mark Sanders, 5 years ago

Hi oromit (and cehoyos),

I review the code, and finally I prepared a patch:
https://patchwork.ffmpeg.org/patch/11165/

It resolves the issue.

I hope it will be soon accepted.
Regards.
M.Sanders.

Note: See TracTickets for help on using tickets.