Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: Fallback to duration if sample rate is unavailable

[Michael Niedermayer]

On Wed, May 02, 2018 at 02:57:11AM +0200, wm4 wrote:
> On Tue,  1 May 2018 22:44:07 +0200
> Michael Niedermayer  wrote:
> 
> > Regression since: af1761f7
> > Fixes: Division by 0
> > Fixes: ffmpeg_crash_1
> > 
> > Found-by: Thuan Pham, Marcel Böhme, Andrew Santosa and Alexandru Razvan 
> > Caciulescu with AFLSmart
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  fftools/ffmpeg.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 5dc198f933..15fa54f24e 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -2706,8 +2706,12 @@ static int process_input_packet(InputStream *ist, 
> > const AVPacket *pkt, int no_eo
> >  ist->dts = ist->next_dts;
> >  switch (ist->dec_ctx->codec_type) {
> >  case AVMEDIA_TYPE_AUDIO:
> > -ist->next_dts += ((int64_t)AV_TIME_BASE * 
> > ist->dec_ctx->frame_size) /
> > - ist->dec_ctx->sample_rate;
> > +if (ist->dec_ctx->sample_rate) {
> > +ist->next_dts += ((int64_t)AV_TIME_BASE * 
> > ist->dec_ctx->frame_size) /
> > +  ist->dec_ctx->sample_rate;
> > +} else {
> > +ist->next_dts += av_rescale_q(pkt->duration, 
> > ist->st->time_base, AV_TIME_BASE_Q);
> > +}
> >  break;
> >  case AVMEDIA_TYPE_VIDEO:
> >  if (ist->framerate.num) {
> 
> Wouldn't it be better to error out here, instead of using yet another
> unreliable value and "hoping for the best"?
> 
> Also I'm not sure, but I think unknown durations are sometimes set to
> -1 instead of 0.

The same update this patch would add is also done in the AVMEDIA_TYPE_VIDEO
below the changed AVMEDIA_TYPE_AUDIO.

I can add a special case and error out but then audio and video differ more

about -1, ill add a assert, the existing code prior to the patch would
already misbehave (in some cases) if its -1. So i will have to fix this
even without the patch if it happens

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: Fallback to duration if sample rate is unavailable

[wm4]

On Tue,  1 May 2018 22:44:07 +0200
Michael Niedermayer  wrote:

> Regression since: af1761f7
> Fixes: Division by 0
> Fixes: ffmpeg_crash_1
> 
> Found-by: Thuan Pham, Marcel Böhme, Andrew Santosa and Alexandru Razvan 
> Caciulescu with AFLSmart
> Signed-off-by: Michael Niedermayer 
> ---
>  fftools/ffmpeg.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 5dc198f933..15fa54f24e 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2706,8 +2706,12 @@ static int process_input_packet(InputStream *ist, 
> const AVPacket *pkt, int no_eo
>  ist->dts = ist->next_dts;
>  switch (ist->dec_ctx->codec_type) {
>  case AVMEDIA_TYPE_AUDIO:
> -ist->next_dts += ((int64_t)AV_TIME_BASE * 
> ist->dec_ctx->frame_size) /
> - ist->dec_ctx->sample_rate;
> +if (ist->dec_ctx->sample_rate) {
> +ist->next_dts += ((int64_t)AV_TIME_BASE * 
> ist->dec_ctx->frame_size) /
> +  ist->dec_ctx->sample_rate;
> +} else {
> +ist->next_dts += av_rescale_q(pkt->duration, 
> ist->st->time_base, AV_TIME_BASE_Q);
> +}
>  break;
>  case AVMEDIA_TYPE_VIDEO:
>  if (ist->framerate.num) {

Wouldn't it be better to error out here, instead of using yet another
unreliable value and "hoping for the best"?

Also I'm not sure, but I think unknown durations are sometimes set to
-1 instead of 0.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fftools/ffmpeg: Fallback to duration if sample rate is unavailable

[Michael Niedermayer]

Regression since: af1761f7
Fixes: Division by 0
Fixes: ffmpeg_crash_1

Found-by: Thuan Pham, Marcel Böhme, Andrew Santosa and Alexandru Razvan 
Caciulescu with AFLSmart
Signed-off-by: Michael Niedermayer 
---
 fftools/ffmpeg.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 5dc198f933..15fa54f24e 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2706,8 +2706,12 @@ static int process_input_packet(InputStream *ist, const 
AVPacket *pkt, int no_eo
 ist->dts = ist->next_dts;
 switch (ist->dec_ctx->codec_type) {
 case AVMEDIA_TYPE_AUDIO:
-ist->next_dts += ((int64_t)AV_TIME_BASE * 
ist->dec_ctx->frame_size) /
- ist->dec_ctx->sample_rate;
+if (ist->dec_ctx->sample_rate) {
+ist->next_dts += ((int64_t)AV_TIME_BASE * 
ist->dec_ctx->frame_size) /
+  ist->dec_ctx->sample_rate;
+} else {
+ist->next_dts += av_rescale_q(pkt->duration, 
ist->st->time_base, AV_TIME_BASE_Q);
+}
 break;
 case AVMEDIA_TYPE_VIDEO:
 if (ist->framerate.num) {
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel