mbox series

[FFmpeg-devel,v3,0/6] ffmpeg: late A/V encoder init, AVFrame metadata usage

Message ID 20201016131649.4361-1-jeebjp@gmail.com
Headers show
Series ffmpeg: late A/V encoder init, AVFrame metadata usage | expand

Message

Jan Ekström Oct. 16, 2020, 1:16 p.m. UTC
This patch set started with a very simple wish to not have to set color
related values manually each time when utilizing ffmpeg.c.

As of the third iteration, the following changes were done since the second:
1. A simple mistake was corrected, fixing `debug_ts`.
2. As I noticed such a change enabling a fix for the interlaced flag writing
   for Y4M, switched the location of the field order and interlaced/progressive
   logic to where the encoder is initialized.
3. First attempt at fixing cases where the difference between stream copy
   and re-encoding leads to the muxer queue filling up, breaking cases
   where a stream with lots of small packets (such as audio) is copied,
   and a seek ends up multiple seconds before the actual requested seek
   time.

Unfortunately, audio still needs two locations where the encoder is
initialized, due to how avfilter_graph_request_oldest peeks and already puts
one AVFrame to be available from the filter graph (which is then utilized
as-is as an early return inside both av_buffersink_get_frame_flags and
av_buffersink_get_samples). If this would be improved in lavfi (or the call
to avfilter_graph_request_oldest removed), we could at least remove one of
these.

Currently limited to using values for video and started with the basic values,
more can be added later if needed.

This probably fixes some trac issues, but with a quick look I couldn't find
anything that explicitly was due to lack of video color metadata passthrough.


Jan


Example 1:

I have an RGB 3-D render, which I would like to encode into BT.709 YCbCr.
The video filter I'm generally using for this (zscale) does flag the matrix in
the output AVFrame.

Yet to have the video encoder have the correct metadata set, I have to
set the value(s) manually.

With this patch set, the value(s) from the first AVFrame fed to do_video_out
will be utilized.

Example 2:

I have an input video that sets one or more of the following:
matrix/primaries/transfer function/range/chroma location.

I just want to re-encode it. All of this metadata gets stripped.

With this patch set, the value(s) from the first AVFrame fed to do_video_out
will be utilized.

Example 3:

I have a video which has incorrect metadata tagged. Before, I had to set
the correct data data manually.

With this patch set, since ffmpeg.c takes color related options as dictionary
keys, the AVFrame values will only be utilized if the user has not set the
option for a given stream. Thus, this use case still works.


Jan Ekström (6):
  ffmpeg: deduplicate init_output_stream usage logic
  ffmpeg: move AVFrame time base adjustment into a function
  ffmpeg: move A/V non-streamcopy initialization to a later point
  ffmpeg: pass decoded or filtered AVFrame to output stream
    initialization
  ffmpeg: move field order decision making to encoder initialization
  ffmpeg: add a data size threshold for muxing queue size

 doc/ffmpeg.texi                               |   5 +
 fftools/ffmpeg.c                              | 249 ++++++++++++------
 fftools/ffmpeg.h                              |  11 +
 fftools/ffmpeg_opt.c                          |   8 +
 .../fate/concat-demuxer-extended-lavf-mxf_d10 |   2 +-
 .../fate/concat-demuxer-simple1-lavf-mxf_d10  |   2 +-
 tests/ref/fate/rgb24-mkv                      |   4 +-
 tests/ref/lavf/mxf_d10                        |   2 +-
 tests/ref/lavf/mxf_dv25                       |   2 +-
 tests/ref/lavf/mxf_dvcpro50                   |   2 +-
 tests/ref/lavf/mxf_opatom                     |   2 +-
 11 files changed, 202 insertions(+), 87 deletions(-)

Comments

Jan Ekström Oct. 21, 2020, 7:14 a.m. UTC | #1
On Fri, Oct 16, 2020, 16:16 Jan Ekström <jeebjp@gmail.com> wrote:

> This patch set started with a very simple wish to not have to set color
> related values manually each time when utilizing ffmpeg.c.
>
> As of the third iteration, the following changes were done since the
> second:
> 1. A simple mistake was corrected, fixing `debug_ts`.
> 2. As I noticed such a change enabling a fix for the interlaced flag
> writing
>    for Y4M, switched the location of the field order and
> interlaced/progressive
>    logic to where the encoder is initialized.
> 3. First attempt at fixing cases where the difference between stream copy
>    and re-encoding leads to the muxer queue filling up, breaking cases
>    where a stream with lots of small packets (such as audio) is copied,
>    and a seek ends up multiple seconds before the actual requested seek
>    time.
>
> Unfortunately, audio still needs two locations where the encoder is
> initialized, due to how avfilter_graph_request_oldest peeks and already
> puts
> one AVFrame to be available from the filter graph (which is then utilized
> as-is as an early return inside both av_buffersink_get_frame_flags and
> av_buffersink_get_samples). If this would be improved in lavfi (or the call
> to avfilter_graph_request_oldest removed), we could at least remove one of
> these.
>
> Currently limited to using values for video and started with the basic
> values,
> more can be added later if needed.
>
> This probably fixes some trac issues, but with a quick look I couldn't find
> anything that explicitly was due to lack of video color metadata
> passthrough.
>
>
> Jan
>
>
> Example 1:
>
> I have an RGB 3-D render, which I would like to encode into BT.709 YCbCr.
> The video filter I'm generally using for this (zscale) does flag the
> matrix in
> the output AVFrame.
>
> Yet to have the video encoder have the correct metadata set, I have to
> set the value(s) manually.
>
> With this patch set, the value(s) from the first AVFrame fed to
> do_video_out
> will be utilized.
>
> Example 2:
>
> I have an input video that sets one or more of the following:
> matrix/primaries/transfer function/range/chroma location.
>
> I just want to re-encode it. All of this metadata gets stripped.
>
> With this patch set, the value(s) from the first AVFrame fed to
> do_video_out
> will be utilized.
>
> Example 3:
>
> I have a video which has incorrect metadata tagged. Before, I had to set
> the correct data data manually.
>
> With this patch set, since ffmpeg.c takes color related options as
> dictionary
> keys, the AVFrame values will only be utilized if the user has not set the
> option for a given stream. Thus, this use case still works.
>
>
> Jan Ekström (6):
>   ffmpeg: deduplicate init_output_stream usage logic
>   ffmpeg: move AVFrame time base adjustment into a function
>   ffmpeg: move A/V non-streamcopy initialization to a later point
>   ffmpeg: pass decoded or filtered AVFrame to output stream
>     initialization
>   ffmpeg: move field order decision making to encoder initialization
>   ffmpeg: add a data size threshold for muxing queue size
>
>  doc/ffmpeg.texi                               |   5 +
>  fftools/ffmpeg.c                              | 249 ++++++++++++------
>  fftools/ffmpeg.h                              |  11 +
>  fftools/ffmpeg_opt.c                          |   8 +
>  .../fate/concat-demuxer-extended-lavf-mxf_d10 |   2 +-
>  .../fate/concat-demuxer-simple1-lavf-mxf_d10  |   2 +-
>  tests/ref/fate/rgb24-mkv                      |   4 +-
>  tests/ref/lavf/mxf_d10                        |   2 +-
>  tests/ref/lavf/mxf_dv25                       |   2 +-
>  tests/ref/lavf/mxf_dvcpro50                   |   2 +-
>  tests/ref/lavf/mxf_opatom                     |   2 +-
>  11 files changed, 202 insertions(+), 87 deletions(-)
>
> --
> 2.26.2
>

Ping on the patch set or if nothing else on the patch that improves the mux
queue buffer?

As far as I can see this can enable passing through information such as
stereo3d info as long as we get the info through AVFrame side data, but I
do not want to add even more changes to this patch set until it has
proceeded at least somewhat.

Jan

>
Jan Ekström Oct. 25, 2020, 12:22 p.m. UTC | #2
On Wed, Oct 21, 2020 at 10:14 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Fri, Oct 16, 2020, 16:16 Jan Ekström <jeebjp@gmail.com> wrote:
>>
>> This patch set started with a very simple wish to not have to set color
>> related values manually each time when utilizing ffmpeg.c.
>>
>> As of the third iteration, the following changes were done since the second:
>> 1. A simple mistake was corrected, fixing `debug_ts`.
>> 2. As I noticed such a change enabling a fix for the interlaced flag writing
>>    for Y4M, switched the location of the field order and interlaced/progressive
>>    logic to where the encoder is initialized.
>> 3. First attempt at fixing cases where the difference between stream copy
>>    and re-encoding leads to the muxer queue filling up, breaking cases
>>    where a stream with lots of small packets (such as audio) is copied,
>>    and a seek ends up multiple seconds before the actual requested seek
>>    time.
>>
>> Unfortunately, audio still needs two locations where the encoder is
>> initialized, due to how avfilter_graph_request_oldest peeks and already puts
>> one AVFrame to be available from the filter graph (which is then utilized
>> as-is as an early return inside both av_buffersink_get_frame_flags and
>> av_buffersink_get_samples). If this would be improved in lavfi (or the call
>> to avfilter_graph_request_oldest removed), we could at least remove one of
>> these.
>>
>> Currently limited to using values for video and started with the basic values,
>> more can be added later if needed.
>>
>> This probably fixes some trac issues, but with a quick look I couldn't find
>> anything that explicitly was due to lack of video color metadata passthrough.
>>
>>
>> Jan
>>
>>
>> Example 1:
>>
>> I have an RGB 3-D render, which I would like to encode into BT.709 YCbCr.
>> The video filter I'm generally using for this (zscale) does flag the matrix in
>> the output AVFrame.
>>
>> Yet to have the video encoder have the correct metadata set, I have to
>> set the value(s) manually.
>>
>> With this patch set, the value(s) from the first AVFrame fed to do_video_out
>> will be utilized.
>>
>> Example 2:
>>
>> I have an input video that sets one or more of the following:
>> matrix/primaries/transfer function/range/chroma location.
>>
>> I just want to re-encode it. All of this metadata gets stripped.
>>
>> With this patch set, the value(s) from the first AVFrame fed to do_video_out
>> will be utilized.
>>
>> Example 3:
>>
>> I have a video which has incorrect metadata tagged. Before, I had to set
>> the correct data data manually.
>>
>> With this patch set, since ffmpeg.c takes color related options as dictionary
>> keys, the AVFrame values will only be utilized if the user has not set the
>> option for a given stream. Thus, this use case still works.
>>
>>
>> Jan Ekström (6):
>>   ffmpeg: deduplicate init_output_stream usage logic
>>   ffmpeg: move AVFrame time base adjustment into a function
>>   ffmpeg: move A/V non-streamcopy initialization to a later point
>>   ffmpeg: pass decoded or filtered AVFrame to output stream
>>     initialization
>>   ffmpeg: move field order decision making to encoder initialization
>>   ffmpeg: add a data size threshold for muxing queue size
>>
>>  doc/ffmpeg.texi                               |   5 +
>>  fftools/ffmpeg.c                              | 249 ++++++++++++------
>>  fftools/ffmpeg.h                              |  11 +
>>  fftools/ffmpeg_opt.c                          |   8 +
>>  .../fate/concat-demuxer-extended-lavf-mxf_d10 |   2 +-
>>  .../fate/concat-demuxer-simple1-lavf-mxf_d10  |   2 +-
>>  tests/ref/fate/rgb24-mkv                      |   4 +-
>>  tests/ref/lavf/mxf_d10                        |   2 +-
>>  tests/ref/lavf/mxf_dv25                       |   2 +-
>>  tests/ref/lavf/mxf_dvcpro50                   |   2 +-
>>  tests/ref/lavf/mxf_opatom                     |   2 +-
>>  11 files changed, 202 insertions(+), 87 deletions(-)
>>
>> --
>> 2.26.2
>
>
> Ping on the patch set or if nothing else on the patch that improves the mux queue buffer?
>
> As far as I can see this can enable passing through information such as stereo3d info as long as we get the info through AVFrame side data, but I do not want to add even more changes to this patch set until it has proceeded at least somewhat.

ping²

Jan