| Message ID | 20190803204024.4593-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 686755f02b0214155f3a044f263c2b60abdf9800 |
| Headers | show |
Aug 3, 2019, 9:40 PM by cus@passwd.hu: > Otherwise the user might get a silence padded frame in the beginning or in the > middle of the encoding. > > Some other bug uncovered this: > > ./ffmpeg -loglevel verbose -y -f data -i /dev/zero \ > -filter_complex "nullsrc=s=60x60:d=10[v0];sine=d=10[a]" \ > -map '[v0]' -c:v:0 rawvideo \ > -map '[a]' -c:a:0 mp2 \ > -f mpegts out.ts > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavcodec/encode.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/encode.c b/libavcodec/encode.c > index d12c42526b..d81b32b983 100644 > --- a/libavcodec/encode.c > +++ b/libavcodec/encode.c > @@ -174,8 +174,14 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, > goto end; > } > } else if (!(avctx->codec->capabilities & AV_CODEC_CAP_VARIABLE_FRAME_SIZE)) { > - if (frame->nb_samples < avctx->frame_size && > - !avctx->internal->last_audio_frame) { > + /* if we already got an undersized frame, that must have been the last */ > + if (avctx->internal->last_audio_frame) { > + av_log(avctx, AV_LOG_ERROR, "frame_size (%d) was not respected for a non-last frame (avcodec_encode_audio2)\n", avctx->frame_size); > + ret = AVERROR(EINVAL); > + goto end; > + } > + > + if (frame->nb_samples < avctx->frame_size) { > ret = pad_last_frame(avctx, &padded_frame, frame); > if (ret < 0) > goto end; > You haven't fixed the bug, you've just made it an error. You have to fix the encoder instead. The check might be useful if its made a warning instead though.
On Sat, 3 Aug 2019, Lynne wrote: > Aug 3, 2019, 9:40 PM by cus@passwd.hu: > >> Otherwise the user might get a silence padded frame in the beginning or in the >> middle of the encoding. >> >> Some other bug uncovered this: >> >> ./ffmpeg -loglevel verbose -y -f data -i /dev/zero \ >> -filter_complex "nullsrc=s=60x60:d=10[v0];sine=d=10[a]" \ >> -map '[v0]' -c:v:0 rawvideo \ >> -map '[a]' -c:a:0 mp2 \ >> -f mpegts out.ts >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavcodec/encode.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/libavcodec/encode.c b/libavcodec/encode.c >> index d12c42526b..d81b32b983 100644 >> --- a/libavcodec/encode.c >> +++ b/libavcodec/encode.c >> @@ -174,8 +174,14 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, >> goto end; >> } >> } else if (!(avctx->codec->capabilities & AV_CODEC_CAP_VARIABLE_FRAME_SIZE)) { >> - if (frame->nb_samples < avctx->frame_size && >> - !avctx->internal->last_audio_frame) { >> + /* if we already got an undersized frame, that must have been the last */ >> + if (avctx->internal->last_audio_frame) { >> + av_log(avctx, AV_LOG_ERROR, "frame_size (%d) was not respected for a non-last frame (avcodec_encode_audio2)\n", avctx->frame_size); >> + ret = AVERROR(EINVAL); >> + goto end; >> + } >> + >> + if (frame->nb_samples < avctx->frame_size) { >> ret = pad_last_frame(avctx, &padded_frame, frame); >> if (ret < 0) >> goto end; >> > > You haven't fixed the bug, you've just made it an error. You have to fix the encoder instead. > The check might be useful if its made a warning instead though. The command line was also failing before the patch with an invalid frame size error at the end of the stream. So ffmpeg.c / libavfilter not providing fixed frame sizes is a different bug to the one this patch supposed to fix. This fix disallows padding a frame with silence in the middle of the encoding if the user provides an undersized frame. libavcodec should reject such encoding attempts and not corrupt the output with silence. Regards, Marton
On Sun, 4 Aug 2019, Marton Balint wrote: > > > On Sat, 3 Aug 2019, Lynne wrote: > >> Aug 3, 2019, 9:40 PM by cus@passwd.hu: >> >>> Otherwise the user might get a silence padded frame in the beginning or in > the >>> middle of the encoding. >>> >>> Some other bug uncovered this: >>> >>> ./ffmpeg -loglevel verbose -y -f data -i /dev/zero \ >>> -filter_complex "nullsrc=s=60x60:d=10[v0];sine=d=10[a]" \ >>> -map '[v0]' -c:v:0 rawvideo \ >>> -map '[a]' -c:a:0 mp2 \ >>> -f mpegts out.ts >>> >>> Signed-off-by: Marton Balint <cus@passwd.hu> >>> --- >>> libavcodec/encode.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c >>> index d12c42526b..d81b32b983 100644 >>> --- a/libavcodec/encode.c >>> +++ b/libavcodec/encode.c >>> @@ -174,8 +174,14 @@ int attribute_align_arg > avcodec_encode_audio2(AVCodecContext *avctx, >>> goto end; >>> } >>> } else if (!(avctx->codec->capabilities & > AV_CODEC_CAP_VARIABLE_FRAME_SIZE)) { >>> - if (frame->nb_samples < avctx->frame_size && >>> - !avctx->internal->last_audio_frame) { >>> + /* if we already got an undersized frame, that must have been > the last */ >>> + if (avctx->internal->last_audio_frame) { >>> + av_log(avctx, AV_LOG_ERROR, "frame_size (%d) was not > respected for a non-last frame (avcodec_encode_audio2)\n", > avctx->frame_size); >>> + ret = AVERROR(EINVAL); >>> + goto end; >>> + } >>> + >>> + if (frame->nb_samples < avctx->frame_size) { >>> ret = pad_last_frame(avctx, &padded_frame, frame); >>> if (ret < 0) >>> goto end; >>> >> >> You haven't fixed the bug, you've just made it an error. You have to fix > the encoder instead. >> The check might be useful if its made a warning instead though. > > The command line was also failing before the patch with an invalid frame > size error at the end of the stream. So ffmpeg.c / libavfilter not > providing fixed frame sizes is a different bug to the one this patch > supposed to fix. > > This fix disallows padding a frame with silence in the middle of the > encoding if the user provides an undersized frame. libavcodec should > reject such encoding attempts and not corrupt the output with silence. > Ping, will apply soon. Regards, Marton
On Sat, 10 Aug 2019, Marton Balint wrote: > > > On Sun, 4 Aug 2019, Marton Balint wrote: > >> >> >> On Sat, 3 Aug 2019, Lynne wrote: >> >>> Aug 3, 2019, 9:40 PM by cus@passwd.hu: >>> >>>> Otherwise the user might get a silence padded frame in the beginning or > in >> the >>>> middle of the encoding. >>>> >>>> Some other bug uncovered this: >>>> >>>> ./ffmpeg -loglevel verbose -y -f data -i /dev/zero \ >>>> -filter_complex "nullsrc=s=60x60:d=10[v0];sine=d=10[a]" \ >>>> -map '[v0]' -c:v:0 rawvideo \ >>>> -map '[a]' -c:a:0 mp2 \ >>>> -f mpegts out.ts >>>> >>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>> --- >>>> libavcodec/encode.c | 10 ++++++++-- >>>> 1 file changed, 8 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c >>>> index d12c42526b..d81b32b983 100644 >>>> --- a/libavcodec/encode.c >>>> +++ b/libavcodec/encode.c >>>> @@ -174,8 +174,14 @@ int attribute_align_arg >> avcodec_encode_audio2(AVCodecContext *avctx, >>>> goto end; >>>> } >>>> } else if (!(avctx->codec->capabilities & >> AV_CODEC_CAP_VARIABLE_FRAME_SIZE)) { >>>> - if (frame->nb_samples < avctx->frame_size && >>>> - !avctx->internal->last_audio_frame) { >>>> + /* if we already got an undersized frame, that must have > been >> the last */ >>>> + if (avctx->internal->last_audio_frame) { >>>> + av_log(avctx, AV_LOG_ERROR, "frame_size (%d) was not >> respected for a non-last frame (avcodec_encode_audio2)\n", >> avctx->frame_size); >>>> + ret = AVERROR(EINVAL); >>>> + goto end; >>>> + } >>>> + >>>> + if (frame->nb_samples < avctx->frame_size) { >>>> ret = pad_last_frame(avctx, &padded_frame, frame); >>>> if (ret < 0) >>>> goto end; >>>> >>> >>> You haven't fixed the bug, you've just made it an error. You have to fix >> the encoder instead. >>> The check might be useful if its made a warning instead though. >> >> The command line was also failing before the patch with an invalid frame >> size error at the end of the stream. So ffmpeg.c / libavfilter not >> providing fixed frame sizes is a different bug to the one this patch >> supposed to fix. >> >> This fix disallows padding a frame with silence in the middle of the >> encoding if the user provides an undersized frame. libavcodec should >> reject such encoding attempts and not corrupt the output with silence. >> > > Ping, will apply soon. Applied. Regards, Marton
diff --git a/libavcodec/encode.c b/libavcodec/encode.c index d12c42526b..d81b32b983 100644 --- a/libavcodec/encode.c +++ b/libavcodec/encode.c @@ -174,8 +174,14 @@ int attribute_align_arg avcodec_encode_audio2(AVCodecContext *avctx, goto end; } } else if (!(avctx->codec->capabilities & AV_CODEC_CAP_VARIABLE_FRAME_SIZE)) { - if (frame->nb_samples < avctx->frame_size && - !avctx->internal->last_audio_frame) { + /* if we already got an undersized frame, that must have been the last */ + if (avctx->internal->last_audio_frame) { + av_log(avctx, AV_LOG_ERROR, "frame_size (%d) was not respected for a non-last frame (avcodec_encode_audio2)\n", avctx->frame_size); + ret = AVERROR(EINVAL); + goto end; + } + + if (frame->nb_samples < avctx->frame_size) { ret = pad_last_frame(avctx, &padded_frame, frame); if (ret < 0) goto end;
Otherwise the user might get a silence padded frame in the beginning or in the middle of the encoding. Some other bug uncovered this: ./ffmpeg -loglevel verbose -y -f data -i /dev/zero \ -filter_complex "nullsrc=s=60x60:d=10[v0];sine=d=10[a]" \ -map '[v0]' -c:v:0 rawvideo \ -map '[a]' -c:a:0 mp2 \ -f mpegts out.ts Signed-off-by: Marton Balint <cus@passwd.hu> --- libavcodec/encode.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)