mbox series

[FFmpeg-devel,v6,0/5] Fix mpeg1/2 stream copy

Message ID 20200113185459.8908-1-nicolas.gaullier@cji.paris
Headers show
Series Fix mpeg1/2 stream copy | expand

Message

Nicolas Gaullier Jan. 13, 2020, 6:54 p.m. UTC
This patch was initially composed of 7 patchs, the two first have been applied.
There was no feedback on these remaining 5 patchs for long, but I just have catched in
the patchwork that Michael asked me to repost the whole set to facilitate the review...
So here it is!
Thank you in advance for the review.

For the record, the target use case is to fix things like this:
  ffmpeg -i xdcamhd.mxf -c:v copy output.mpg

(And note that as a side effect, ffmpeg now reports the cpb and signaled bitrate of mpeg2 video inputs)
Nicolas

Nicolas Gaullier (5):
  avformat: Add av_stream_add_coded_side_data()
  avformat/utils: Make find_stream_info get side data from codec context
  fftools/ffmpeg: Use the new av_stream_add_coded_side_data()
  avcodec/utils: Fix ff_add_cpb_side_data() add twice
  avcodec/mpeg12dec: Add CPB coded side data

 doc/APIchanges               |  3 +++
 fftools/ffmpeg.c             | 16 +++-------------
 libavcodec/mpeg12dec.c       |  7 +++++++
 libavcodec/utils.c           |  5 +++++
 libavformat/avformat.h       | 11 +++++++++++
 libavformat/utils.c          | 18 ++++++++++++++++++
 libavformat/version.h        |  4 ++--
 tests/ref/fate/mxf-probe-d10 |  3 +++
 tests/ref/fate/ts-demux      |  2 +-
 9 files changed, 53 insertions(+), 16 deletions(-)

Comments

Carl Eugen Hoyos Jan. 13, 2020, 9:02 p.m. UTC | #1
Am Mo., 13. Jan. 2020 um 19:55 Uhr schrieb Nicolas Gaullier
<nicolas.gaullier@cji.paris>:
>
> This patch was initially composed of 7 patchs, the two first have been applied.
> There was no feedback on these remaining 5 patchs for long, but I just have catched in
> the patchwork that Michael asked me to repost the whole set to facilitate the review...

Apart from what Anton wrote, it appears to me that not all patches in the
series are necessary to fix the stream-copying.

Carl Eugen
Nicolas Gaullier Jan. 13, 2020, 10:42 p.m. UTC | #2
>Apart from what Anton wrote, it appears to me that not all patches in the
>series are necessary to fix the stream-copying.
>
>Carl Eugen

Precisely:
- 1/2/3 are working together : actual code from ffmpeg.c that is used for stream encoding is now used for probing in libav, so it is "moved to public" and shared. But strictly speaking, 3 is not required to fix the stream-copying, it is only a "code simplification" = use the newly created public method.
- 4 is a bug fix that is necessary to apply before 5 to avoid generating duplicate side data (this does not prevent stream copy from working, but of course this is very bad), but maybe I should have indicated this in the commit msg
- 5 just add the coded side data that find_stream_info will forward to stream side data that ffmpeg will forward to output mux in case of stream copy.

Sorry, I have little experience in contributing to open projects and I think I tend to not document enough in the commit msgs

Nicolas