Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Michael Niedermayer]

On Wed, Sep 30, 2015 at 04:32:24PM +0200, Hendrik Leppkes wrote:
> On Wed, Sep 30, 2015 at 2:51 PM, Michael Niedermayer  wrote:
> > On Wed, Sep 30, 2015 at 02:39:21PM +0200, Hendrik Leppkes wrote:
> >> On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer  
> >> wrote:
> >> > On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
> >> >> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer  
> >> >> wrote:
> >> >> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
> >> >> >> The chroma format can be still unset in postinit when a badly cut 
> >> >> >> stream
> >> >> >> starts with a slice instead of a sequence header. This is a common
> >> >> >> occurance when feeding avcodec from a Live TV stream.
> >> >> >> ---
> >> >> >>  libavcodec/mpeg12dec.c | 1 -
> >> >> >>  1 file changed, 1 deletion(-)
> >> >> >>
> >> >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> >> >> >> index 5d916d1..b3c2c45 100644
> >> >> >> --- a/libavcodec/mpeg12dec.c
> >> >> >> +++ b/libavcodec/mpeg12dec.c
> >> >> >> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext 
> >> >> >> *avctx)
> >> >> >>  case 1: avctx->chroma_sample_location = 
> >> >> >> AVCHROMA_LOC_LEFT; break;
> >> >> >>  case 2:
> >> >> >>  case 3: avctx->chroma_sample_location = 
> >> >> >> AVCHROMA_LOC_TOPLEFT; break;
> >> >> >> -default: av_assert0(0);
> >> >> >>  }
> >> >> >>  } // MPEG-2
> >> >> >
> >> >> > This assert double checks that the context init which uses
> >> >> > width/height/chroma format is done after the chroma format and w/h has
> >> >> > been read from the headers
> >> >> > if this is reached without the headers being read then the code is
> >> >> > buggy and removing the assert will not fix that bug
> >> >> >
> >> >> > also if there is no sequence header how is width/height set ?
> >> >> > theres a check for width/height before mpeg_decode_postinit is called
> >> >> >
> >> >>
> >> >> The dimensions are set by the caller in the avctx before opening the
> >> >> codec, which apparently inits the mpeg context as well.
> >> >>
> >> >> Feel free to fix it differently, error out or something, but I can
> >> >> trip an assert with input data, which should never happen.
> >> >
> >> > how can this be reproduced ?
> >> >
> >>
> >> I can only trigger it with API usage, not through the CLI tools, so I
> >> cannot produce a test case.
> >
> > is this with a unmodified up to date ffmpeg ?
> > i fixed this assert failing a while ago 
> > (b54e03c9dc2a05324c08b503bfe7535c49c0f281)
> >
> >
> 
> Its up to date, but slightly modified - but nothing pertinent to the
> mpeg2 decoder as far as I know.
> I'll just add this to the list of modifications then. I can clearly
> trigger this assert in a release build (which is hilarious on its own,
> asserts are a debugging tool), which crashes the application, so thats
> no good.

having the context inconsistent and fields at invalid values is also
not good.
Ill try to fix it

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

No snowflake in an avalanche ever feels responsible. -- Voltaire


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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Paul B Mahol]

On 9/30/15, Hendrik Leppkes  wrote:
> The chroma format can be still unset in postinit when a badly cut stream
> starts with a slice instead of a sequence header. This is a common
> occurance when feeding avcodec from a Live TV stream.
> ---
>  libavcodec/mpeg12dec.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 5d916d1..b3c2c45 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext *avctx)
>  case 1: avctx->chroma_sample_location = AVCHROMA_LOC_LEFT;
> break;
>  case 2:
>  case 3: avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT;
> break;
> -default: av_assert0(0);
>  }
>  } // MPEG-2
>
> --
> 2.5.3.windows.1

There could be warning of some sort, lgtm anyway.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Michael Niedermayer]

On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer  wrote:
> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
> >> The chroma format can be still unset in postinit when a badly cut stream
> >> starts with a slice instead of a sequence header. This is a common
> >> occurance when feeding avcodec from a Live TV stream.
> >> ---
> >>  libavcodec/mpeg12dec.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> >> index 5d916d1..b3c2c45 100644
> >> --- a/libavcodec/mpeg12dec.c
> >> +++ b/libavcodec/mpeg12dec.c
> >> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext 
> >> *avctx)
> >>  case 1: avctx->chroma_sample_location = AVCHROMA_LOC_LEFT; 
> >> break;
> >>  case 2:
> >>  case 3: avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; 
> >> break;
> >> -default: av_assert0(0);
> >>  }
> >>  } // MPEG-2
> >
> > This assert double checks that the context init which uses
> > width/height/chroma format is done after the chroma format and w/h has
> > been read from the headers
> > if this is reached without the headers being read then the code is
> > buggy and removing the assert will not fix that bug
> >
> > also if there is no sequence header how is width/height set ?
> > theres a check for width/height before mpeg_decode_postinit is called
> >
> 
> The dimensions are set by the caller in the avctx before opening the
> codec, which apparently inits the mpeg context as well.
> 
> Feel free to fix it differently, error out or something, but I can
> trip an assert with input data, which should never happen.

how can this be reproduced ?

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Michael Niedermayer]

On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
> The chroma format can be still unset in postinit when a badly cut stream
> starts with a slice instead of a sequence header. This is a common
> occurance when feeding avcodec from a Live TV stream.
> ---
>  libavcodec/mpeg12dec.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 5d916d1..b3c2c45 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext *avctx)
>  case 1: avctx->chroma_sample_location = AVCHROMA_LOC_LEFT; break;
>  case 2:
>  case 3: avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; 
> break;
> -default: av_assert0(0);
>  }
>  } // MPEG-2

This assert double checks that the context init which uses
width/height/chroma format is done after the chroma format and w/h has
been read from the headers
if this is reached without the headers being read then the code is
buggy and removing the assert will not fix that bug

also if there is no sequence header how is width/height set ?
theres a check for width/height before mpeg_decode_postinit is called

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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact


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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Hendrik Leppkes]

On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer  wrote:
> On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>> The chroma format can be still unset in postinit when a badly cut stream
>> starts with a slice instead of a sequence header. This is a common
>> occurance when feeding avcodec from a Live TV stream.
>> ---
>>  libavcodec/mpeg12dec.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>> index 5d916d1..b3c2c45 100644
>> --- a/libavcodec/mpeg12dec.c
>> +++ b/libavcodec/mpeg12dec.c
>> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext *avctx)
>>  case 1: avctx->chroma_sample_location = AVCHROMA_LOC_LEFT; 
>> break;
>>  case 2:
>>  case 3: avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; 
>> break;
>> -default: av_assert0(0);
>>  }
>>  } // MPEG-2
>
> This assert double checks that the context init which uses
> width/height/chroma format is done after the chroma format and w/h has
> been read from the headers
> if this is reached without the headers being read then the code is
> buggy and removing the assert will not fix that bug
>
> also if there is no sequence header how is width/height set ?
> theres a check for width/height before mpeg_decode_postinit is called
>

The dimensions are set by the caller in the avctx before opening the
codec, which apparently inits the mpeg context as well.

Feel free to fix it differently, error out or something, but I can
trip an assert with input data, which should never happen.

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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Hendrik Leppkes]

On Wed, Sep 30, 2015 at 2:29 PM, Hendrik Leppkes  wrote:
> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer  wrote:
>> On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>>> The chroma format can be still unset in postinit when a badly cut stream
>>> starts with a slice instead of a sequence header. This is a common
>>> occurance when feeding avcodec from a Live TV stream.
>>> ---
>>>  libavcodec/mpeg12dec.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>>> index 5d916d1..b3c2c45 100644
>>> --- a/libavcodec/mpeg12dec.c
>>> +++ b/libavcodec/mpeg12dec.c
>>> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext *avctx)
>>>  case 1: avctx->chroma_sample_location = AVCHROMA_LOC_LEFT; 
>>> break;
>>>  case 2:
>>>  case 3: avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; 
>>> break;
>>> -default: av_assert0(0);
>>>  }
>>>  } // MPEG-2
>>
>> This assert double checks that the context init which uses
>> width/height/chroma format is done after the chroma format and w/h has
>> been read from the headers
>> if this is reached without the headers being read then the code is
>> buggy and removing the assert will not fix that bug
>>
>> also if there is no sequence header how is width/height set ?
>> theres a check for width/height before mpeg_decode_postinit is called
>>
>
> The dimensions are set by the caller in the avctx before opening the
> codec, which apparently inits the mpeg context as well.
>
> Feel free to fix it differently, error out or something, but I can
> trip an assert with input data, which should never happen.

Just to amend - after the fix it works perfectly fine again. It errors
out on a bunch of frames of course later on, but then it fixes itself
and decodes the Live TV stream like usual.

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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Hendrik Leppkes]

On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer  wrote:
> On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
>> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer  
>> wrote:
>> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>> >> The chroma format can be still unset in postinit when a badly cut stream
>> >> starts with a slice instead of a sequence header. This is a common
>> >> occurance when feeding avcodec from a Live TV stream.
>> >> ---
>> >>  libavcodec/mpeg12dec.c | 1 -
>> >>  1 file changed, 1 deletion(-)
>> >>
>> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>> >> index 5d916d1..b3c2c45 100644
>> >> --- a/libavcodec/mpeg12dec.c
>> >> +++ b/libavcodec/mpeg12dec.c
>> >> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext 
>> >> *avctx)
>> >>  case 1: avctx->chroma_sample_location = AVCHROMA_LOC_LEFT; 
>> >> break;
>> >>  case 2:
>> >>  case 3: avctx->chroma_sample_location = 
>> >> AVCHROMA_LOC_TOPLEFT; break;
>> >> -default: av_assert0(0);
>> >>  }
>> >>  } // MPEG-2
>> >
>> > This assert double checks that the context init which uses
>> > width/height/chroma format is done after the chroma format and w/h has
>> > been read from the headers
>> > if this is reached without the headers being read then the code is
>> > buggy and removing the assert will not fix that bug
>> >
>> > also if there is no sequence header how is width/height set ?
>> > theres a check for width/height before mpeg_decode_postinit is called
>> >
>>
>> The dimensions are set by the caller in the avctx before opening the
>> codec, which apparently inits the mpeg context as well.
>>
>> Feel free to fix it differently, error out or something, but I can
>> trip an assert with input data, which should never happen.
>
> how can this be reproduced ?
>

I can only trigger it with API usage, not through the CLI tools, so I
cannot produce a test case.
Just replace the assert with a error return, that should cover everything.

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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Paul B Mahol]

On 9/30/15, Hendrik Leppkes  wrote:
> On Wed, Sep 30, 2015 at 2:51 PM, Michael Niedermayer 
> wrote:
>> On Wed, Sep 30, 2015 at 02:39:21PM +0200, Hendrik Leppkes wrote:
>>> On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer 
>>> wrote:
>>> > On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
>>> >> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer
>>> >>  wrote:
>>> >> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>>> >> >> The chroma format can be still unset in postinit when a badly cut
>>> >> >> stream
>>> >> >> starts with a slice instead of a sequence header. This is a common
>>> >> >> occurance when feeding avcodec from a Live TV stream.
>>> >> >> ---
>>> >> >>  libavcodec/mpeg12dec.c | 1 -
>>> >> >>  1 file changed, 1 deletion(-)
>>> >> >>
>>> >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>>> >> >> index 5d916d1..b3c2c45 100644
>>> >> >> --- a/libavcodec/mpeg12dec.c
>>> >> >> +++ b/libavcodec/mpeg12dec.c
>>> >> >> @@ -1389,7 +1389,6 @@ static int
>>> >> >> mpeg_decode_postinit(AVCodecContext *avctx)
>>> >> >>  case 1: avctx->chroma_sample_location =
>>> >> >> AVCHROMA_LOC_LEFT; break;
>>> >> >>  case 2:
>>> >> >>  case 3: avctx->chroma_sample_location =
>>> >> >> AVCHROMA_LOC_TOPLEFT; break;
>>> >> >> -default: av_assert0(0);
>>> >> >>  }
>>> >> >>  } // MPEG-2
>>> >> >
>>> >> > This assert double checks that the context init which uses
>>> >> > width/height/chroma format is done after the chroma format and w/h
>>> >> > has
>>> >> > been read from the headers
>>> >> > if this is reached without the headers being read then the code is
>>> >> > buggy and removing the assert will not fix that bug
>>> >> >
>>> >> > also if there is no sequence header how is width/height set ?
>>> >> > theres a check for width/height before mpeg_decode_postinit is
>>> >> > called
>>> >> >
>>> >>
>>> >> The dimensions are set by the caller in the avctx before opening the
>>> >> codec, which apparently inits the mpeg context as well.
>>> >>
>>> >> Feel free to fix it differently, error out or something, but I can
>>> >> trip an assert with input data, which should never happen.
>>> >
>>> > how can this be reproduced ?
>>> >
>>>
>>> I can only trigger it with API usage, not through the CLI tools, so I
>>> cannot produce a test case.
>>
>> is this with a unmodified up to date ffmpeg ?
>> i fixed this assert failing a while ago
>> (b54e03c9dc2a05324c08b503bfe7535c49c0f281)
>>
>>
>
> Its up to date, but slightly modified - but nothing pertinent to the
> mpeg2 decoder as far as I know.
> I'll just add this to the list of modifications then. I can clearly
> trigger this assert in a release build (which is hilarious on its own,
> asserts are a debugging tool), which crashes the application, so thats
> no good.

WTF, can you just give sample/link something?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Hendrik Leppkes]

On Wed, Sep 30, 2015 at 2:51 PM, Michael Niedermayer  wrote:
> On Wed, Sep 30, 2015 at 02:39:21PM +0200, Hendrik Leppkes wrote:
>> On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer  
>> wrote:
>> > On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
>> >> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer  
>> >> wrote:
>> >> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>> >> >> The chroma format can be still unset in postinit when a badly cut 
>> >> >> stream
>> >> >> starts with a slice instead of a sequence header. This is a common
>> >> >> occurance when feeding avcodec from a Live TV stream.
>> >> >> ---
>> >> >>  libavcodec/mpeg12dec.c | 1 -
>> >> >>  1 file changed, 1 deletion(-)
>> >> >>
>> >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>> >> >> index 5d916d1..b3c2c45 100644
>> >> >> --- a/libavcodec/mpeg12dec.c
>> >> >> +++ b/libavcodec/mpeg12dec.c
>> >> >> @@ -1389,7 +1389,6 @@ static int mpeg_decode_postinit(AVCodecContext 
>> >> >> *avctx)
>> >> >>  case 1: avctx->chroma_sample_location = 
>> >> >> AVCHROMA_LOC_LEFT; break;
>> >> >>  case 2:
>> >> >>  case 3: avctx->chroma_sample_location = 
>> >> >> AVCHROMA_LOC_TOPLEFT; break;
>> >> >> -default: av_assert0(0);
>> >> >>  }
>> >> >>  } // MPEG-2
>> >> >
>> >> > This assert double checks that the context init which uses
>> >> > width/height/chroma format is done after the chroma format and w/h has
>> >> > been read from the headers
>> >> > if this is reached without the headers being read then the code is
>> >> > buggy and removing the assert will not fix that bug
>> >> >
>> >> > also if there is no sequence header how is width/height set ?
>> >> > theres a check for width/height before mpeg_decode_postinit is called
>> >> >
>> >>
>> >> The dimensions are set by the caller in the avctx before opening the
>> >> codec, which apparently inits the mpeg context as well.
>> >>
>> >> Feel free to fix it differently, error out or something, but I can
>> >> trip an assert with input data, which should never happen.
>> >
>> > how can this be reproduced ?
>> >
>>
>> I can only trigger it with API usage, not through the CLI tools, so I
>> cannot produce a test case.
>
> is this with a unmodified up to date ffmpeg ?
> i fixed this assert failing a while ago 
> (b54e03c9dc2a05324c08b503bfe7535c49c0f281)
>
>

Its up to date, but slightly modified - but nothing pertinent to the
mpeg2 decoder as far as I know.
I'll just add this to the list of modifications then. I can clearly
trigger this assert in a release build (which is hilarious on its own,
asserts are a debugging tool), which crashes the application, so thats
no good.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Paul B Mahol]

Dana 30. 9. 2015. 17:19 osoba "Hendrik Leppkes" 
napisala je:
>
> On Wed, Sep 30, 2015 at 4:35 PM, Paul B Mahol  wrote:
> > On 9/30/15, Hendrik Leppkes  wrote:
> >> On Wed, Sep 30, 2015 at 2:51 PM, Michael Niedermayer 
> >> wrote:
> >>> On Wed, Sep 30, 2015 at 02:39:21PM +0200, Hendrik Leppkes wrote:
>  On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer <
michae...@gmx.at>
>  wrote:
>  > On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
>  >> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer
>  >>  wrote:
>  >> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>  >> >> The chroma format can be still unset in postinit when a badly
cut
>  >> >> stream
>  >> >> starts with a slice instead of a sequence header. This is a
common
>  >> >> occurance when feeding avcodec from a Live TV stream.
>  >> >> ---
>  >> >>  libavcodec/mpeg12dec.c | 1 -
>  >> >>  1 file changed, 1 deletion(-)
>  >> >>
>  >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>  >> >> index 5d916d1..b3c2c45 100644
>  >> >> --- a/libavcodec/mpeg12dec.c
>  >> >> +++ b/libavcodec/mpeg12dec.c
>  >> >> @@ -1389,7 +1389,6 @@ static int
>  >> >> mpeg_decode_postinit(AVCodecContext *avctx)
>  >> >>  case 1: avctx->chroma_sample_location =
>  >> >> AVCHROMA_LOC_LEFT; break;
>  >> >>  case 2:
>  >> >>  case 3: avctx->chroma_sample_location =
>  >> >> AVCHROMA_LOC_TOPLEFT; break;
>  >> >> -default: av_assert0(0);
>  >> >>  }
>  >> >>  } // MPEG-2
>  >> >
>  >> > This assert double checks that the context init which uses
>  >> > width/height/chroma format is done after the chroma format and
w/h
>  >> > has
>  >> > been read from the headers
>  >> > if this is reached without the headers being read then the code
is
>  >> > buggy and removing the assert will not fix that bug
>  >> >
>  >> > also if there is no sequence header how is width/height set ?
>  >> > theres a check for width/height before mpeg_decode_postinit is
>  >> > called
>  >> >
>  >>
>  >> The dimensions are set by the caller in the avctx before opening
the
>  >> codec, which apparently inits the mpeg context as well.
>  >>
>  >> Feel free to fix it differently, error out or something, but I can
>  >> trip an assert with input data, which should never happen.
>  >
>  > how can this be reproduced ?
>  >
> 
>  I can only trigger it with API usage, not through the CLI tools, so I
>  cannot produce a test case.
> >>>
> >>> is this with a unmodified up to date ffmpeg ?
> >>> i fixed this assert failing a while ago
> >>> (b54e03c9dc2a05324c08b503bfe7535c49c0f281)
> >>>
> >>>
> >>
> >> Its up to date, but slightly modified - but nothing pertinent to the
> >> mpeg2 decoder as far as I know.
> >> I'll just add this to the list of modifications then. I can clearly
> >> trigger this assert in a release build (which is hilarious on its own,
> >> asserts are a debugging tool), which crashes the application, so thats
> >> no good.
> >
> > WTF, can you just give sample/link something?
>
> Like I said earlier in the thread, it happens with API usage in a Live
> TV scenario. I cannot produce a sample of that.

Not even backtrace?

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

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Hendrik Leppkes]

On Wed, Sep 30, 2015 at 6:03 PM, Paul B Mahol  wrote:
> Dana 30. 9. 2015. 17:19 osoba "Hendrik Leppkes" 
> napisala je:
>>
>> On Wed, Sep 30, 2015 at 4:35 PM, Paul B Mahol  wrote:
>> > On 9/30/15, Hendrik Leppkes  wrote:
>> >> On Wed, Sep 30, 2015 at 2:51 PM, Michael Niedermayer 
>> >> wrote:
>> >>> On Wed, Sep 30, 2015 at 02:39:21PM +0200, Hendrik Leppkes wrote:
>>  On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer <
> michae...@gmx.at>
>>  wrote:
>>  > On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
>>  >> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer
>>  >>  wrote:
>>  >> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
>>  >> >> The chroma format can be still unset in postinit when a badly
> cut
>>  >> >> stream
>>  >> >> starts with a slice instead of a sequence header. This is a
> common
>>  >> >> occurance when feeding avcodec from a Live TV stream.
>>  >> >> ---
>>  >> >>  libavcodec/mpeg12dec.c | 1 -
>>  >> >>  1 file changed, 1 deletion(-)
>>  >> >>
>>  >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
>>  >> >> index 5d916d1..b3c2c45 100644
>>  >> >> --- a/libavcodec/mpeg12dec.c
>>  >> >> +++ b/libavcodec/mpeg12dec.c
>>  >> >> @@ -1389,7 +1389,6 @@ static int
>>  >> >> mpeg_decode_postinit(AVCodecContext *avctx)
>>  >> >>  case 1: avctx->chroma_sample_location =
>>  >> >> AVCHROMA_LOC_LEFT; break;
>>  >> >>  case 2:
>>  >> >>  case 3: avctx->chroma_sample_location =
>>  >> >> AVCHROMA_LOC_TOPLEFT; break;
>>  >> >> -default: av_assert0(0);
>>  >> >>  }
>>  >> >>  } // MPEG-2
>>  >> >
>>  >> > This assert double checks that the context init which uses
>>  >> > width/height/chroma format is done after the chroma format and
> w/h
>>  >> > has
>>  >> > been read from the headers
>>  >> > if this is reached without the headers being read then the code
> is
>>  >> > buggy and removing the assert will not fix that bug
>>  >> >
>>  >> > also if there is no sequence header how is width/height set ?
>>  >> > theres a check for width/height before mpeg_decode_postinit is
>>  >> > called
>>  >> >
>>  >>
>>  >> The dimensions are set by the caller in the avctx before opening
> the
>>  >> codec, which apparently inits the mpeg context as well.
>>  >>
>>  >> Feel free to fix it differently, error out or something, but I can
>>  >> trip an assert with input data, which should never happen.
>>  >
>>  > how can this be reproduced ?
>>  >
>> 
>>  I can only trigger it with API usage, not through the CLI tools, so I
>>  cannot produce a test case.
>> >>>
>> >>> is this with a unmodified up to date ffmpeg ?
>> >>> i fixed this assert failing a while ago
>> >>> (b54e03c9dc2a05324c08b503bfe7535c49c0f281)
>> >>>
>> >>>
>> >>
>> >> Its up to date, but slightly modified - but nothing pertinent to the
>> >> mpeg2 decoder as far as I know.
>> >> I'll just add this to the list of modifications then. I can clearly
>> >> trigger this assert in a release build (which is hilarious on its own,
>> >> asserts are a debugging tool), which crashes the application, so thats
>> >> no good.
>> >
>> > WTF, can you just give sample/link something?
>>
>> Like I said earlier in the thread, it happens with API usage in a Live
>> TV scenario. I cannot produce a sample of that.
>
> Not even backtrace?
>

postinit is only called from one place, the trace isn't very interesting.

mpeg_decode_postinit
decode_chunks
mpeg_decode_frame
avcodec_decode_video2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mpeg12dec: don't assert on unknown chroma format

[Hendrik Leppkes]

On Wed, Sep 30, 2015 at 4:35 PM, Paul B Mahol  wrote:
> On 9/30/15, Hendrik Leppkes  wrote:
>> On Wed, Sep 30, 2015 at 2:51 PM, Michael Niedermayer 
>> wrote:
>>> On Wed, Sep 30, 2015 at 02:39:21PM +0200, Hendrik Leppkes wrote:
 On Wed, Sep 30, 2015 at 2:33 PM, Michael Niedermayer 
 wrote:
 > On Wed, Sep 30, 2015 at 02:29:43PM +0200, Hendrik Leppkes wrote:
 >> On Wed, Sep 30, 2015 at 2:10 PM, Michael Niedermayer
 >>  wrote:
 >> > On Wed, Sep 30, 2015 at 12:42:59PM +0200, Hendrik Leppkes wrote:
 >> >> The chroma format can be still unset in postinit when a badly cut
 >> >> stream
 >> >> starts with a slice instead of a sequence header. This is a common
 >> >> occurance when feeding avcodec from a Live TV stream.
 >> >> ---
 >> >>  libavcodec/mpeg12dec.c | 1 -
 >> >>  1 file changed, 1 deletion(-)
 >> >>
 >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
 >> >> index 5d916d1..b3c2c45 100644
 >> >> --- a/libavcodec/mpeg12dec.c
 >> >> +++ b/libavcodec/mpeg12dec.c
 >> >> @@ -1389,7 +1389,6 @@ static int
 >> >> mpeg_decode_postinit(AVCodecContext *avctx)
 >> >>  case 1: avctx->chroma_sample_location =
 >> >> AVCHROMA_LOC_LEFT; break;
 >> >>  case 2:
 >> >>  case 3: avctx->chroma_sample_location =
 >> >> AVCHROMA_LOC_TOPLEFT; break;
 >> >> -default: av_assert0(0);
 >> >>  }
 >> >>  } // MPEG-2
 >> >
 >> > This assert double checks that the context init which uses
 >> > width/height/chroma format is done after the chroma format and w/h
 >> > has
 >> > been read from the headers
 >> > if this is reached without the headers being read then the code is
 >> > buggy and removing the assert will not fix that bug
 >> >
 >> > also if there is no sequence header how is width/height set ?
 >> > theres a check for width/height before mpeg_decode_postinit is
 >> > called
 >> >
 >>
 >> The dimensions are set by the caller in the avctx before opening the
 >> codec, which apparently inits the mpeg context as well.
 >>
 >> Feel free to fix it differently, error out or something, but I can
 >> trip an assert with input data, which should never happen.
 >
 > how can this be reproduced ?
 >

 I can only trigger it with API usage, not through the CLI tools, so I
 cannot produce a test case.
>>>
>>> is this with a unmodified up to date ffmpeg ?
>>> i fixed this assert failing a while ago
>>> (b54e03c9dc2a05324c08b503bfe7535c49c0f281)
>>>
>>>
>>
>> Its up to date, but slightly modified - but nothing pertinent to the
>> mpeg2 decoder as far as I know.
>> I'll just add this to the list of modifications then. I can clearly
>> trigger this assert in a release build (which is hilarious on its own,
>> asserts are a debugging tool), which crashes the application, so thats
>> no good.
>
> WTF, can you just give sample/link something?

Like I said earlier in the thread, it happens with API usage in a Live
TV scenario. I cannot produce a sample of that.

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