Re: [FFmpeg-devel] [PATCH] libavformat: add side_data copy in concat demuxer

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Gerard Sole:
> Adds support for concat demuxer to copy the side data information
> from the input file to the resulting file. It will behave like the
> metadata copy, where the metadata of the first file is kept in the
> the output file.
> 
> Extract the current code that already performs the stream side_data
> copy into a separate method and reuse the method in the concat demuxer.
> 
> Signed-off-by: Gerard Sole <g.sole.ca@gmail.com>
> ---
>  libavformat/concatdec.c                             | 1 +
>  libavformat/internal.h                              | 9 +++++++++
>  libavformat/utils.c                                 | 9 +++++++++
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf     | 2 +-
>  tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +-
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf      | 1 +
>  tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10  | 1 +
>  tests/ref/fate/concat-demuxer-simple2-lavf-ts       | 1 +
>  8 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
> index 8d80e536d1..0603c6e254 100644
> --- a/libavformat/concatdec.c
> +++ b/libavformat/concatdec.c
> @@ -191,6 +191,7 @@ static int copy_stream_props(AVStream *st, AVStream *source_st)
>      avpriv_set_pts_info(st, 64, source_st->time_base.num, source_st->time_base.den);
>  
>      av_dict_copy(&st->metadata, source_st->metadata, 0);
> +    ff_stream_side_data_copy(st, source_st);
>      return 0;
>  }
>  
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index eb8239cd3f..0e08dc832e 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -840,6 +840,15 @@ int ff_stream_add_bitstream_filter(AVStream *st, const char *name, const char *a
>   */
>  int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src);
>  
> +/**
> + * Copy side data from source to destination stream
> + *
> + * @param dst pointer to destination AVStream
> + * @param src pointer to source AVStream
> + * @return >=0 on success, AVERROR code on error
> + */
> +int ff_stream_side_data_copy(AVStream *dst, const AVStream *src);
> +
>  /**
>   * Wrap ffurl_move() and log if error happens.
>   *
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index b5a4a09ae8..332ba534d2 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -605,6 +605,15 @@ int ff_stream_encode_params_copy(AVStream *dst, const AVStream *src)
>      if (ret < 0)
>          return ret;
>  
> +    ret = ff_stream_side_data_copy(dst, src);
> +    if (ret < 0)
> +        return ret;
> +
> +    return 0;
> +}
> +
> +int ff_stream_side_data_copy(AVStream *dst, const AVStream *src)
> +{
>      /* Free existing side data*/
>      for (int i = 0; i < dst->nb_side_data; i++)
>          av_free(dst->side_data[i].data);
> diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf b/tests/ref/fate/concat-demuxer-extended-lavf-mxf
> index 4b2a8624db..543c7d6a8c 100644
> --- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf
> +++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf
> @@ -1 +1 @@
> -29e4e502a912b6d863e75d44e156ed31 *tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
> +d367d7f6df7292cbf454c6d07fca9b04 *tests/data/fate/concat-demuxer-extended-lavf-mxf.ffprobe
> diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
> index 1dedc6bf43..57b22848b9 100644
> --- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
> +++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
> @@ -1 +1 @@
> -8de04a786521677a593283c44a53572e *tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
> +1fac6962d4c5f1070d0d2db5ab7d86aa *tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
> diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
> index 2fe703e2a6..589dbb73b6 100644
> --- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
> +++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf
> @@ -121,4 +121,5 @@ Strings Metadata
>  video|0|37|1.480000|34|1.360000|1|0.040000|24786|212480|K_|1
>  Strings Metadata
>  0|mpeg2video|4|video|[0][0][0][0]|0x0000|352|288|0|0|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|progressive|1|N/A|25/1|25/1|1/25|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|51|22|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
> +CPB properties|0|0|0|49152|-1
>  1|pcm_s16le|unknown|audio|[0][0][0][0]|0x0000|s16|48000|1|unknown|16|N/A|0/0|0/0|1/48000|0|0.000000|N/A|N/A|768000|N/A|N/A|N/A|N/A|50|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
> diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
> index 0a3af658f6..7fb6ba2c9c 100644
> --- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
> +++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
> @@ -79,4 +79,5 @@ Strings Metadata
>  audio|1|65280|1.360000|65280|1.360000|1920|0.040000|7680|2074624|K_|1
>  Strings Metadata
>  0|mpeg2video|0|video|[0][0][0][0]|0x0000|720|608|0|0|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|tb|1|N/A|25/1|25/1|1/25|0|0.000000|N/A|N/A|30000000|N/A|N/A|N/A|N/A|35|22|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
> +CPB properties|30000000|0|0|1212416|-1
>  1|pcm_s16le|unknown|audio|[0][0][0][0]|0x0000|s16|48000|2|unknown|16|N/A|0/0|0/0|1/48000|0|0.000000|N/A|N/A|1536000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001300000000000000000000000000000000000001
> diff --git a/tests/ref/fate/concat-demuxer-simple2-lavf-ts b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
> index 76dde3b873..7c6079e2b1 100644
> --- a/tests/ref/fate/concat-demuxer-simple2-lavf-ts
> +++ b/tests/ref/fate/concat-demuxer-simple2-lavf-ts
> @@ -213,3 +213,4 @@ video|1|175582|1.950911|171982|1.910911|3600|0.040000|15019|224848|__MPEGTS Stre
>  
>  0|mp2|unknown|audio|[3][0][0][0]|0x0003|s16p|44100|1|mono|0|N/A|0/0|0/0|1/90000|0|0.000000|N/A|N/A|64000|N/A|N/A|N/A|N/A|89|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|this is stream 0
>  1|mpeg2video|4|video|[2][0][0][0]|0x0002|352|288|0|0|0|0|1|1:1|11:9|yuv420p|8|tv|unknown|unknown|unknown|left|progressive|1|N/A|25/1|25/1|1/90000|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|N/A|60|22|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|this is stream 1
> +CPB properties|0|0|0|49152|-1
> 

The documentation of AVStream.side_data contains: "demuxing: Set by
libavformat when the stream is created". Does your patch guarantee this?
copy_stream_props() seems reachable from concat_read_packet().
(Actually, a similar question can be asked about extradata and other
AVCodecParameters fields.)

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".