[FFmpeg-devel] [PATCH 1/2] avformat/yuv4mpegenc: Simplify writing global and packet headers

[Paul B Mahol]

On 9/4/20, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
> YUV4MPEG writes a string as header for both the file itself as well as
> for every frame; these strings contain magic strings and these were up
> until now included in the string to write via %s. Yet they are compile
> time constants, so one can use the compile-time string concatentation
> instead of inserting these strings at runtime.
> Furthermore, the global header has been written via snprintf() to
> a local buffer first before writing it. This can be simplified by using
> avio_printf().
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---


lgtm

> That snprintf() call was weird: The buffer used had a size of
> Y4M_LINE_MAX + 1, yet snprintf has been told that the size was
> Y4M_LINE_MAX, despite snprintf always adding a trailing zero (i.e. it
> writes at most Y4M_LINE_MAX - 1 chars and then adds a zero).
> Furthermore, snprintf only returns something negative on format errors,
> not if the buffer is too small (this can be checked via the return
> value). And returning AVERROR(EIO) for this error is strange, too.
>
>  libavformat/yuv4mpegenc.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/libavformat/yuv4mpegenc.c b/libavformat/yuv4mpegenc.c
> index c4781042bd..fdd020e13b 100644
> --- a/libavformat/yuv4mpegenc.c
> +++ b/libavformat/yuv4mpegenc.c
> @@ -24,18 +24,15 @@
>  #include "internal.h"
>  #include "yuv4mpeg.h"
>
> -#define Y4M_LINE_MAX 256
> -
>  static int yuv4_write_header(AVFormatContext *s)
>  {
>      AVStream *st;
>      AVIOContext *pb = s->pb;
>      int width, height;
> -    int raten, rated, aspectn, aspectd, n;
> +    int raten, rated, aspectn, aspectd, ret;
>      char inter;
>      const char *colorspace = "";
>      const char *colorrange = "";
> -    char buf[Y4M_LINE_MAX + 1];
>      int field_order;
>
>      st     = s->streams[0];
> @@ -170,19 +167,15 @@ static int yuv4_write_header(AVFormatContext *s)
>          break;
>      }
>
> -    /* construct stream header, if this is the first frame */
> -    n = snprintf(buf, Y4M_LINE_MAX, "%s W%d H%d F%d:%d I%c A%d:%d%s%s\n",
> -                 Y4M_MAGIC, width, height, raten, rated, inter,
> -                 aspectn, aspectd, colorspace, colorrange);
> -
> -    if (n < 0) {
> +    ret = avio_printf(pb, Y4M_MAGIC " W%d H%d F%d:%d I%c A%d:%d%s%s\n",
> +                      width, height, raten, rated, inter,
> +                      aspectn, aspectd, colorspace, colorrange);
> +    if (ret < 0) {
>          av_log(s, AV_LOG_ERROR,
>                 "Error. YUV4MPEG stream header write failed.\n");
> -        return AVERROR(EIO);
> +        return ret;
>      }
>
> -    avio_write(pb, buf, strlen(buf));
> -
>      return 0;
>  }
>
> @@ -200,7 +193,7 @@ static int yuv4_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>
>      /* construct frame header */
>
> -    avio_printf(s->pb, "%s\n", Y4M_FRAME_MAGIC);
> +    avio_printf(s->pb, Y4M_FRAME_MAGIC "\n");
>
>      width  = st->codecpar->width;
>      height = st->codecpar->height;
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".