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-requ...@ffmpeg.org with subject "unsubscribe".
