[FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Stefano Sabatini]

Provide useful information about the failure in the error message, do not let the user
guess.
---
 libavformat/dvenc.c | 119 +++++++++++++++++++++++++++++++-------------
 1 file changed, 85 insertions(+), 34 deletions(-)

diff --git a/libavformat/dvenc.c b/libavformat/dvenc.c
index 29d2dc47ac..9a89b8e4f5 100644
--- a/libavformat/dvenc.c
+++ b/libavformat/dvenc.c
@@ -39,6 +39,7 @@
 #include "libavutil/fifo.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/intreadwrite.h"
+#include "libavutil/pixdesc.h"
 #include "libavutil/timecode.h"
 
 #define MAX_AUDIO_FRAME_SIZE 192000 // 1 second of 48khz 32-bit audio
@@ -308,62 +309,112 @@ static int dv_assemble_frame(AVFormatContext *s,
     return 0;
 }
 
-static DVMuxContext* dv_init_mux(AVFormatContext* s)
+static int dv_init_mux(AVFormatContext* s)
 {
     DVMuxContext *c = s->priv_data;
     AVStream *vst = NULL;
     int i;
 
-    /* we support at most 1 video and 2 audio streams */
-    if (s->nb_streams > 5)
-        return NULL;
+    if (s->nb_streams > 5) {
+        av_log(s, AV_LOG_ERROR,
+               "Invalid number of streams %d, the muxer supports at most 1 video channel and 4 audio channels.\n",
+               s->nb_streams);
+        return AVERROR_INVALIDDATA;
+    }
 
     /* We have to sort out where audio and where video stream is */
     for (i=0; i<s->nb_streams; i++) {
         AVStream *st = s->streams[i];
         switch (st->codecpar->codec_type) {
         case AVMEDIA_TYPE_VIDEO:
-            if (vst) return NULL;
-            if (st->codecpar->codec_id != AV_CODEC_ID_DVVIDEO)
-                goto bail_out;
+            if (vst) {
+                av_log(s, AV_LOG_ERROR,
+                       "More than one video stream found, only one is accepted.\n");
+                return AVERROR_INVALIDDATA;
+            }
+            if (st->codecpar->codec_id != AV_CODEC_ID_DVVIDEO) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid codec for video stream, only DVVIDEO is supported.\n");
+                return AVERROR_INVALIDDATA;
+            }
             vst = st;
             break;
         case AVMEDIA_TYPE_AUDIO:
-            if (c->n_ast > 1) return NULL;
+            if (c->n_ast > 1) {
+                av_log(s, AV_LOG_ERROR,
+                       "More than two audio streams found, at most 2 are accepted.\n");
+                return AVERROR_INVALIDDATA;
+            }
             /* Some checks -- DV format is very picky about its incoming streams */
-            if(st->codecpar->codec_id    != AV_CODEC_ID_PCM_S16LE ||
-               st->codecpar->ch_layout.nb_channels    != 2)
-                goto bail_out;
+            if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid codec for stream %d, only PCM_S16LE is supported\n.", i);
+                return AVERROR_INVALIDDATA;
+            }
+            if (st->codecpar->ch_layout.nb_channels != 2) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid number of audio channels %d for stream %d, only 2 channels are supported\n.",
+                       st->codecpar->ch_layout.nb_channels, i);
+                return AVERROR_INVALIDDATA;
+            }
             if (st->codecpar->sample_rate != 48000 &&
                 st->codecpar->sample_rate != 44100 &&
-                st->codecpar->sample_rate != 32000    )
-                goto bail_out;
+                st->codecpar->sample_rate != 32000) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid audio sample rate %d for stream %d, only 32000, 44100, and 48000 are supported.\n",
+                       st->codecpar->sample_rate, i);
+                return AVERROR_INVALIDDATA;
+            }
             c->ast[c->n_ast++] = st;
             break;
         default:
-            goto bail_out;
+            av_log(s, AV_LOG_ERROR,
+                   "Invalid media type for stream %d, only audio and video are supported.\n", i);
+            return AVERROR_INVALIDDATA;
         }
     }
 
-    if (!vst)
-        goto bail_out;
+    if (!vst) {
+        av_log(s, AV_LOG_ERROR,
+               "Missing video stream, must be present\n");
+        return AVERROR_INVALIDDATA;
+    }
 
     c->sys = av_dv_codec_profile2(vst->codecpar->width, vst->codecpar->height,
                                   vst->codecpar->format, vst->time_base);
-    if (!c->sys)
-        goto bail_out;
+    if (!c->sys) {
+        av_log(s, AV_LOG_ERROR,
+               "Could not find a valid video profile for size:%dx%d format:%s tb:%d%d\n",
+               vst->codecpar->width, vst->codecpar->height,
+               av_get_pix_fmt_name(vst->codecpar->format),
+               vst->time_base.num, vst->time_base.den);
+        return AVERROR_INVALIDDATA;
+    }
 
     if ((c->sys->time_base.den != 25 && c->sys->time_base.den != 50) || c->sys->time_base.num != 1) {
-        if (c->ast[0] && c->ast[0]->codecpar->sample_rate != 48000)
-            goto bail_out;
-        if (c->ast[1] && c->ast[1]->codecpar->sample_rate != 48000)
-            goto bail_out;
+        int j;
+
+        for (j = 0; j < 2; j++) {
+            if (c->ast[j] && c->ast[j]->codecpar->sample_rate != 48000) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid sample rate %d for audio stream #%d for this video profile, must be 48000.\n",
+                       c->ast[j]->codecpar->sample_rate, j);
+                return AVERROR_INVALIDDATA;
+            }
+        }
     }
 
-    if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) ||
-        ((c->n_ast > 2) && (c->sys->n_difchan < 4))) {
-        /* only 2 stereo pairs allowed in 50Mbps mode */
-        goto bail_out;
+    if ((c->n_ast > 1) && (c->sys->n_difchan < 2)) {
+        av_log(s, AV_LOG_ERROR,
+               "Invalid number of channels %d, only 1 stereo pairs is allowed in 25Mps mode.\n",
+               c->n_ast);
+        return AVERROR_INVALIDDATA;
+    }
+    if ((c->n_ast > 2) && (c->sys->n_difchan < 4)) {
+        av_log(s, AV_LOG_ERROR,
+               "Invalid number of channels %d, only 2 stereo pairs are allowed in 50Mps mode.\n",
+               c->n_ast);
+        return AVERROR_INVALIDDATA;
     }
 
     /* Ok, everything seems to be in working order */
@@ -376,14 +427,14 @@ static DVMuxContext* dv_init_mux(AVFormatContext* s)
         if (!c->ast[i])
            continue;
         c->audio_data[i] = av_fifo_alloc2(100 * MAX_AUDIO_FRAME_SIZE, 1, 0);
-        if (!c->audio_data[i])
-            goto bail_out;
+        if (!c->audio_data[i]) {
+            av_log(s, AV_LOG_ERROR,
+                   "Failed to allocate internal buffer.\n");
+            return AVERROR(ENOMEM);
+        }
     }
 
-    return c;
-
-bail_out:
-    return NULL;
+    return 0;
 }
 
 static int dv_write_header(AVFormatContext *s)
@@ -392,10 +443,10 @@ static int dv_write_header(AVFormatContext *s)
     DVMuxContext *dvc = s->priv_data;
     AVDictionaryEntry *tcr = av_dict_get(s->metadata, "timecode", NULL, 0);
 
-    if (!dv_init_mux(s)) {
+    if (dv_init_mux(s) < 0) {
         av_log(s, AV_LOG_ERROR, "Can't initialize DV format!\n"
                     "Make sure that you supply exactly two streams:\n"
-                    "     video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n"
+                    "     video: 25fps or 29.97fps, audio: 2ch/48000|44100|32000Hz/PCM\n"
                     "     (50Mbps allows an optional second audio stream)\n");
         return -1;
     }
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/2] doc/muxers: add dv

[Stefano Sabatini]

---
 doc/muxers.texi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 24ed1b3369..a8bc642c00 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1339,6 +1339,20 @@ of each audio packet, with a size computed according to the formula:
 The @var{encoded_sample_size} = 3 factor is due to sample size of the
 @samp{pcm_24daud} encoder.
 
+@section dv
+DV (Digital Video) muxer.
+
+It accepts exactly one @samp{dvvideo} video stream and at most two
+@samp{pcm_s16} audio streams. More constraints are defined by the
+property of the video, which must correspond to a DV video supported
+profile, and on the framerate.
+
+@subsection Example
+Use @command{ffmpeg} to convert the input:
+@example
+ffmpeg -i INPUT -s:v 720x480 -pix_fmt yuv411p -r 29.97 -ac 2 -ar 48000 -y out.dv
+@end example
+
 @anchor{fifo}
 @section fifo
 
-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-20 16:24:07)
>      if ((c->sys->time_base.den != 25 && c->sys->time_base.den != 50) || c->sys->time_base.num != 1) {
> -        if (c->ast[0] && c->ast[0]->codecpar->sample_rate != 48000)
> -            goto bail_out;
> -        if (c->ast[1] && c->ast[1]->codecpar->sample_rate != 48000)
> -            goto bail_out;
> +        int j;

No need to declare a loop variable outside of the loop. Also, there's
already i.

> -    if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) ||
> -        ((c->n_ast > 2) && (c->sys->n_difchan < 4))) {
> -        /* only 2 stereo pairs allowed in 50Mbps mode */
> -        goto bail_out;
> +    if ((c->n_ast > 1) && (c->sys->n_difchan < 2)) {
> +        av_log(s, AV_LOG_ERROR,
> +               "Invalid number of channels %d, only 1 stereo pairs is allowed in 25Mps mode.\n",
> +               c->n_ast);
> +        return AVERROR_INVALIDDATA;
> +    }
> +    if ((c->n_ast > 2) && (c->sys->n_difchan < 4)) {
> +        av_log(s, AV_LOG_ERROR,
> +               "Invalid number of channels %d, only 2 stereo pairs are allowed in 50Mps mode.\n",
> +               c->n_ast);
> +        return AVERROR_INVALIDDATA;

Surely this can be done in one log statement.

>      }
>  
>      /* Ok, everything seems to be in working order */
> @@ -376,14 +427,14 @@ static DVMuxContext* dv_init_mux(AVFormatContext* s)
>          if (!c->ast[i])
>             continue;
>          c->audio_data[i] = av_fifo_alloc2(100 * MAX_AUDIO_FRAME_SIZE, 1, 0);
> -        if (!c->audio_data[i])
> -            goto bail_out;
> +        if (!c->audio_data[i]) {
> +            av_log(s, AV_LOG_ERROR,
> +                   "Failed to allocate internal buffer.\n");

Dedicated log messages for small malloc failures are useless bloat.

> +            return AVERROR(ENOMEM);
> +        }
>      }
>  
> -    return c;
> -
> -bail_out:
> -    return NULL;
> +    return 0;
>  }
>  
>  static int dv_write_header(AVFormatContext *s)
> @@ -392,10 +443,10 @@ static int dv_write_header(AVFormatContext *s)
>      DVMuxContext *dvc = s->priv_data;
>      AVDictionaryEntry *tcr = av_dict_get(s->metadata, "timecode", NULL, 0);
>  
> -    if (!dv_init_mux(s)) {
> +    if (dv_init_mux(s) < 0) {
>          av_log(s, AV_LOG_ERROR, "Can't initialize DV format!\n"
>                      "Make sure that you supply exactly two streams:\n"

This seems inconsistent with the other checks.

> -                    "     video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n"
> +                    "     video: 25fps or 29.97fps, audio: 2ch/48000|44100|32000Hz/PCM\n"

This does not seem like an improvement.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Stefano Sabatini]

On date Sunday 2024-01-21 07:36:48 +0100, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2024-01-20 16:24:07)
> >      if ((c->sys->time_base.den != 25 && c->sys->time_base.den != 50) || c->sys->time_base.num != 1) {
> > -        if (c->ast[0] && c->ast[0]->codecpar->sample_rate != 48000)
> > -            goto bail_out;
> > -        if (c->ast[1] && c->ast[1]->codecpar->sample_rate != 48000)
> > -            goto bail_out;
> > +        int j;
> 
> No need to declare a loop variable outside of the loop. Also, there's
> already i.

fixed locally

> > -    if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) ||
> > -        ((c->n_ast > 2) && (c->sys->n_difchan < 4))) {
> > -        /* only 2 stereo pairs allowed in 50Mbps mode */
> > -        goto bail_out;
> > +    if ((c->n_ast > 1) && (c->sys->n_difchan < 2)) {
> > +        av_log(s, AV_LOG_ERROR,
> > +               "Invalid number of channels %d, only 1 stereo pairs is allowed in 25Mps mode.\n",
> > +               c->n_ast);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    if ((c->n_ast > 2) && (c->sys->n_difchan < 4)) {
> > +        av_log(s, AV_LOG_ERROR,
> > +               "Invalid number of channels %d, only 2 stereo pairs are allowed in 50Mps mode.\n",
> > +               c->n_ast);
> > +        return AVERROR_INVALIDDATA;
> 

> Surely this can be done in one log statement.

Yes, but this would complicate the logic for small gain.

> 
> >      }
> >  
> >      /* Ok, everything seems to be in working order */
> > @@ -376,14 +427,14 @@ static DVMuxContext* dv_init_mux(AVFormatContext* s)
> >          if (!c->ast[i])
> >             continue;
> >          c->audio_data[i] = av_fifo_alloc2(100 * MAX_AUDIO_FRAME_SIZE, 1, 0);
> > -        if (!c->audio_data[i])
> > -            goto bail_out;
> > +        if (!c->audio_data[i]) {
> > +            av_log(s, AV_LOG_ERROR,
> > +                   "Failed to allocate internal buffer.\n");
> 
> Dedicated log messages for small malloc failures are useless bloat.

Dropped.

> 
> > +            return AVERROR(ENOMEM);
> > +        }
> >      }
> >  
> > -    return c;
> > -
> > -bail_out:
> > -    return NULL;
> > +    return 0;
> >  }
> >  
> >  static int dv_write_header(AVFormatContext *s)
> > @@ -392,10 +443,10 @@ static int dv_write_header(AVFormatContext *s)
> >      DVMuxContext *dvc = s->priv_data;
> >      AVDictionaryEntry *tcr = av_dict_get(s->metadata, "timecode", NULL, 0);
> >  
> > -    if (!dv_init_mux(s)) {
> > +    if (dv_init_mux(s) < 0) {
> >          av_log(s, AV_LOG_ERROR, "Can't initialize DV format!\n"
> >                      "Make sure that you supply exactly two streams:\n"
> 
> This seems inconsistent with the other checks.

Yes, probably it's better to drop this entirely since we have more
puntual reporting now (and "exactly two stream" is wrong) 

> 
> > -                    "     video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n"

> > +                    "     video: 25fps or 29.97fps, audio: 2ch/48000|44100|32000Hz/PCM\n"
> 
> This does not seem like an improvement.

44kHz != 44100

I could use 44.1 but this is not the unit used when setting the
option, so better to be explicit.

Thanks.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-21 11:30:27)
> > > -    if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) ||
> > > -        ((c->n_ast > 2) && (c->sys->n_difchan < 4))) {
> > > -        /* only 2 stereo pairs allowed in 50Mbps mode */
> > > -        goto bail_out;
> > > +    if ((c->n_ast > 1) && (c->sys->n_difchan < 2)) {
> > > +        av_log(s, AV_LOG_ERROR,
> > > +               "Invalid number of channels %d, only 1 stereo pairs is allowed in 25Mps mode.\n",
> > > +               c->n_ast);
> > > +        return AVERROR_INVALIDDATA;
> > > +    }
> > > +    if ((c->n_ast > 2) && (c->sys->n_difchan < 4)) {
> > > +        av_log(s, AV_LOG_ERROR,
> > > +               "Invalid number of channels %d, only 2 stereo pairs are allowed in 50Mps mode.\n",
> > > +               c->n_ast);
> > > +        return AVERROR_INVALIDDATA;
> > 
> 
> > Surely this can be done in one log statement.
> 
> Yes, but this would complicate the logic for small gain.

More complicated than duplicating 5 lines? I wouldn't say so, not to
mention the string also has to be duplicated in the binary.

Also, can the second case even trigger? Seems like the block above
ensures n_ast is never larger than 2.

> > > -                    "     video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n"
> 
> > > +                    "     video: 25fps or 29.97fps, audio: 2ch/48000|44100|32000Hz/PCM\n"
> > 
> > This does not seem like an improvement.
> 
> 44kHz != 44100
> 
> I could use 44.1 but this is not the unit used when setting the
> option

It can be.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Stefano Sabatini]

On date Sunday 2024-01-21 18:39:19 +0100, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2024-01-21 11:30:27)
[...]
> Also, can the second case even trigger? Seems like the block above
> ensures n_ast is never larger than 2.

Yes, this seems a miss from commit
eafa8e859297813dcf11110e6b43e85720be0a5f.

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Stefano Sabatini]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On date Sunday 2024-01-21 20:18:28 +0100, Stefano Sabatini wrote:
> On date Sunday 2024-01-21 18:39:19 +0100, Anton Khirnov wrote:
> > Quoting Stefano Sabatini (2024-01-21 11:30:27)
> [...]
> > Also, can the second case even trigger? Seems like the block above
> > ensures n_ast is never larger than 2.
> 
> Yes, this seems a miss from commit
> eafa8e859297813dcf11110e6b43e85720be0a5f.

Updated, keeping the somehow faulty logic as unrelated to the change
(should be fixed separately).

Adding Baptiste in CC: for the issue with the number of channels.

The obvious fix seems to restore the limit of max 3 channels, and
delete ast[2] and ast[3], but maybe Baptiste can hint at the proper
fix.




[-- Attachment #2: 0001-lavf-dvenc-improve-error-messaging.patch --]
[-- Type: text/x-diff, Size: 7723 bytes --]

From 1027a6b4edb4374c05f10ebe28901e29d8a9291d Mon Sep 17 00:00:00 2001
From: Stefano Sabatini <stefasab@gmail.com>
Date: Sat, 20 Jan 2024 16:17:59 +0100
Subject: [PATCH] lavf/dvenc: improve error messaging

Provide useful information about the failure in the error message, do
not let the user guess.
---
 libavformat/dvenc.c | 116 ++++++++++++++++++++++++++++++--------------
 1 file changed, 80 insertions(+), 36 deletions(-)

diff --git a/libavformat/dvenc.c b/libavformat/dvenc.c
index 29d2dc47ac..0e9a6cfbb1 100644
--- a/libavformat/dvenc.c
+++ b/libavformat/dvenc.c
@@ -39,6 +39,7 @@
 #include "libavutil/fifo.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/intreadwrite.h"
+#include "libavutil/pixdesc.h"
 #include "libavutil/timecode.h"
 
 #define MAX_AUDIO_FRAME_SIZE 192000 // 1 second of 48khz 32-bit audio
@@ -308,62 +309,107 @@ static int dv_assemble_frame(AVFormatContext *s,
     return 0;
 }
 
-static DVMuxContext* dv_init_mux(AVFormatContext* s)
+static int dv_init_mux(AVFormatContext* s)
 {
     DVMuxContext *c = s->priv_data;
     AVStream *vst = NULL;
     int i;
 
-    /* we support at most 1 video and 2 audio streams */
-    if (s->nb_streams > 5)
-        return NULL;
+    if (s->nb_streams > 5) {
+        av_log(s, AV_LOG_ERROR,
+               "Invalid number of streams %d, the muxer supports at most 1 video channel and 4 audio channels.\n",
+               s->nb_streams);
+        return AVERROR_INVALIDDATA;
+    }
 
     /* We have to sort out where audio and where video stream is */
     for (i=0; i<s->nb_streams; i++) {
         AVStream *st = s->streams[i];
         switch (st->codecpar->codec_type) {
         case AVMEDIA_TYPE_VIDEO:
-            if (vst) return NULL;
-            if (st->codecpar->codec_id != AV_CODEC_ID_DVVIDEO)
-                goto bail_out;
+            if (vst) {
+                av_log(s, AV_LOG_ERROR,
+                       "More than one video stream found, only one is accepted.\n");
+                return AVERROR_INVALIDDATA;
+            }
+            if (st->codecpar->codec_id != AV_CODEC_ID_DVVIDEO) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid codec for video stream, only DVVIDEO is supported.\n");
+                return AVERROR_INVALIDDATA;
+            }
             vst = st;
             break;
         case AVMEDIA_TYPE_AUDIO:
-            if (c->n_ast > 1) return NULL;
+            if (c->n_ast > 1) {
+                av_log(s, AV_LOG_ERROR,
+                       "More than two audio streams found, at most 2 are accepted.\n");
+                return AVERROR_INVALIDDATA;
+            }
             /* Some checks -- DV format is very picky about its incoming streams */
-            if(st->codecpar->codec_id    != AV_CODEC_ID_PCM_S16LE ||
-               st->codecpar->ch_layout.nb_channels    != 2)
-                goto bail_out;
+            if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid codec for stream %d, only PCM_S16LE is supported\n.", i);
+                return AVERROR_INVALIDDATA;
+            }
+            if (st->codecpar->ch_layout.nb_channels != 2) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid number of audio channels %d for stream %d, only 2 channels are supported\n.",
+                       st->codecpar->ch_layout.nb_channels, i);
+                return AVERROR_INVALIDDATA;
+            }
             if (st->codecpar->sample_rate != 48000 &&
                 st->codecpar->sample_rate != 44100 &&
-                st->codecpar->sample_rate != 32000    )
-                goto bail_out;
+                st->codecpar->sample_rate != 32000) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid audio sample rate %d for stream %d, only 32000, 44100, and 48000 are supported.\n",
+                       st->codecpar->sample_rate, i);
+                return AVERROR_INVALIDDATA;
+            }
             c->ast[c->n_ast++] = st;
             break;
         default:
-            goto bail_out;
+            av_log(s, AV_LOG_ERROR,
+                   "Invalid media type for stream %d, only audio and video are supported.\n", i);
+            return AVERROR_INVALIDDATA;
         }
     }
 
-    if (!vst)
-        goto bail_out;
+    if (!vst) {
+        av_log(s, AV_LOG_ERROR,
+               "Missing video stream, must be present\n");
+        return AVERROR_INVALIDDATA;
+    }
 
     c->sys = av_dv_codec_profile2(vst->codecpar->width, vst->codecpar->height,
                                   vst->codecpar->format, vst->time_base);
-    if (!c->sys)
-        goto bail_out;
+    if (!c->sys) {
+        av_log(s, AV_LOG_ERROR,
+               "Could not find a valid video profile for size:%dx%d format:%s tb:%d%d\n",
+               vst->codecpar->width, vst->codecpar->height,
+               av_get_pix_fmt_name(vst->codecpar->format),
+               vst->time_base.num, vst->time_base.den);
+        return AVERROR_INVALIDDATA;
+    }
 
     if ((c->sys->time_base.den != 25 && c->sys->time_base.den != 50) || c->sys->time_base.num != 1) {
-        if (c->ast[0] && c->ast[0]->codecpar->sample_rate != 48000)
-            goto bail_out;
-        if (c->ast[1] && c->ast[1]->codecpar->sample_rate != 48000)
-            goto bail_out;
+        for (i = 0; i < 2; i++) {
+            if (c->ast[i] && c->ast[i]->codecpar->sample_rate != 48000) {
+                av_log(s, AV_LOG_ERROR,
+                       "Invalid sample rate %d for audio stream #%d for this video profile, must be 48000.\n",
+                       c->ast[i]->codecpar->sample_rate, i);
+                return AVERROR_INVALIDDATA;
+            }
+        }
     }
 
-    if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) ||
-        ((c->n_ast > 2) && (c->sys->n_difchan < 4))) {
-        /* only 2 stereo pairs allowed in 50Mbps mode */
-        goto bail_out;
+    for (i = 1; i < 2; i++) {
+        if (((c->n_ast > i) && (c->sys->n_difchan < (2 * i)))) {
+            const char *mode = c->n_ast > 1 ? "50Mps" : "25Mps";
+            av_log(s, AV_LOG_ERROR,
+                   "Invalid number of channels %d, only %d stereo pairs is allowed in %s mode.\n",
+                   c->n_ast, i, mode);
+            return AVERROR_INVALIDDATA;
+        }
     }
 
     /* Ok, everything seems to be in working order */
@@ -376,14 +422,12 @@ static DVMuxContext* dv_init_mux(AVFormatContext* s)
         if (!c->ast[i])
            continue;
         c->audio_data[i] = av_fifo_alloc2(100 * MAX_AUDIO_FRAME_SIZE, 1, 0);
-        if (!c->audio_data[i])
-            goto bail_out;
+        if (!c->audio_data[i]) {
+            return AVERROR(ENOMEM);
+        }
     }
 
-    return c;
-
-bail_out:
-    return NULL;
+    return 0;
 }
 
 static int dv_write_header(AVFormatContext *s)
@@ -392,12 +436,12 @@ static int dv_write_header(AVFormatContext *s)
     DVMuxContext *dvc = s->priv_data;
     AVDictionaryEntry *tcr = av_dict_get(s->metadata, "timecode", NULL, 0);
 
-    if (!dv_init_mux(s)) {
+    if (dv_init_mux(s) < 0) {
         av_log(s, AV_LOG_ERROR, "Can't initialize DV format!\n"
-                    "Make sure that you supply exactly two streams:\n"
-                    "     video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n"
+                    "Make sure that you supply at least two streams:\n"
+                    "     video: 25fps or 29.97fps, audio: 2ch/48|44.1|32kHz/PCM\n"
                     "     (50Mbps allows an optional second audio stream)\n");
-        return -1;
+        return AVERROR_INVALIDDATA;
     }
     rate.num = dvc->sys->ltc_divisor;
     rate.den = 1;
-- 
2.34.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".