[FFmpeg-devel] [PATCH 1/3] avformat/matroskaenc: Don't use stream side-data size

[James Almer]

On 5/21/2020 10:24 PM, Andreas Rheinhardt wrote:
> av_stream_get_side_data() tells the caller whether a stream has side
> data of a specific type; if present it can also tell the caller the size
> of the side data via an optional argument. The Matroska muxer always
> used this optional argument, although it doesn't really need the size,
> as the relevant side-data are not buffers, but structures. So change
> this.
> 
> Furthermore, relying on the size also made the code susceptible to
> a quirk of av_stream_get_side_data(): It only sets the size argument if
> it found side data of the desired type.

Sounds like something that should be fixed instead.
av_packet_get_side_data() sets the size argument to 0 if it doesn't find
the requested side data type. This function should do the same.

> mkv_write_video_color() checks
> for side-data twice with the same variable for the size without resetting
> the size in between; if the second type of side-data isn't present, the
> size will still be what it was after the first call. This was not
> dangerous in practice, as the check for the existence of the second
> side-data compared the size with the expected size, so it would only be
> problematic if lots of elements were to be added to AVContentLightMetadata.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavformat/matroskaenc.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index fccfee539a..f5968c17b4 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -835,7 +835,6 @@ static void mkv_write_video_color(AVIOContext *pb, const AVStream *st,
>       * plus another byte to stay clear of the end. */
>      uint8_t colour[(2 + 1 + 8) * 18 + (2 + 1) + 1];
>      AVIOContext buf, *dyn_cp = &buf;
> -    int side_data_size = 0;
>      int colorinfo_size;
>      const uint8_t *side_data;
>  
> @@ -868,8 +867,8 @@ static void mkv_write_video_color(AVIOContext *pb, const AVStream *st,
>      }
>  
>      side_data = av_stream_get_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> -                                        &side_data_size);
> -    if (side_data_size) {
> +                                        NULL);
> +    if (side_data) {
>          const AVContentLightMetadata *metadata =
>              (const AVContentLightMetadata*)side_data;
>          put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXCLL,  metadata->MaxCLL);
> @@ -877,8 +876,8 @@ static void mkv_write_video_color(AVIOContext *pb, const AVStream *st,
>      }
>  
>      side_data = av_stream_get_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
> -                                        &side_data_size);
> -    if (side_data_size == sizeof(AVMasteringDisplayMetadata)) {
> +                                        NULL);
> +    if (side_data) {
>          ebml_master meta_element = start_ebml_master(
>              dyn_cp, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 10 * (2 + 1 + 8));
>          const AVMasteringDisplayMetadata *metadata =
> @@ -919,14 +918,13 @@ static void mkv_write_video_projection(AVFormatContext *s, AVIOContext *pb,
>                                         const AVStream *st)
>  {
>      ebml_master projection;
> -    int side_data_size = 0;
>      uint8_t private[20];
>  
>      const AVSphericalMapping *spherical =
>          (const AVSphericalMapping *)av_stream_get_side_data(st, AV_PKT_DATA_SPHERICAL,
> -                                                            &side_data_size);
> +                                                            NULL);
>  
> -    if (!side_data_size)
> +    if (!spherical)
>          return;
>  
>      if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR      &&
> @@ -1028,7 +1026,6 @@ static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>      const AVDictionaryEntry *tag;
>      MatroskaVideoStereoModeType format = MATROSKA_VIDEO_STEREOMODE_TYPE_NB;
>      const AVStereo3D *stereo;
> -    int side_data_size = 0;
>  
>      *h_width = 1;
>      *h_height = 1;
> @@ -1052,8 +1049,8 @@ static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>      }
>  
>      stereo = (const AVStereo3D*)av_stream_get_side_data(st, AV_PKT_DATA_STEREO3D,
> -                                                        &side_data_size);
> -    if (side_data_size >= sizeof(AVStereo3D)) {
> +                                                        NULL);
> +    if (stereo) {
>          switch (stereo->type) {
>          case AV_STEREO3D_2D:
>              format = MATROSKA_VIDEO_STEREOMODE_TYPE_MONO;
>