Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-17 Thread Mark Thompson 
On 11/12/2018 14:28, Paul B Mahol wrote:
> On 12/11/18, Carl Eugen Hoyos  wrote:
>> 2018-12-10 23:41 GMT+01:00, Mark Thompson :
>>> On 09/12/2018 14:21, Mark Thompson wrote:
 On 09/12/2018 13:54, Paul B Mahol wrote:
> On 12/9/18, Mark Thompson  wrote:
>> On 09/12/2018 08:52, Paul B Mahol wrote:
>>> On 12/7/18, Paul B Mahol  wrote:
 On 12/7/18, Michael Niedermayer  wrote:
> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>> On 12/7/18, Paul B Mahol  wrote:
>>> On 12/7/18, Michael Niedermayer  wrote:
 On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
> This recovers state with #7374 linked sample.
>
> Work funded by Open Broadcast Systems.
>
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/h264_refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index eaf965e43d..5645a203a7 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -718,6 +718,7 @@ int
> ff_h264_execute_ref_pic_marking(H264Context
> *h)
>  }
>  break;
>  case MMCO_RESET:
> +default:
>  while (h->short_ref_count) {
>  remove_short(h, h->short_ref[0]->frame_num, 0);
>  }
> @@ -730,7 +731,6 @@ int
> ff_h264_execute_ref_pic_marking(H264Context
> *h)
>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>  h->last_pocs[j] = INT_MIN;
>  break;
> -default: assert(0);
>  }
>  }

 mmco[i].opcode should not be invalid, its checked around the
 point
 where
 this
 array is filled.
 unless there is something iam missing
>>>
>>> Yes, you are missing big time.
>>> If you think by "checked" about those nice asserts they are not
>>> enabled
>>> at
>>> all.
>>>
>>
>> There is check for invalid opcode, but stored invalid opcode is not
>> changed.
>
> Theres no question that we end with a invalid value in the struct,
> but
> that
> is not intended to be in there. You can see that this is not
> intended
> by
> simply looking at the variable that holds the number of entries, it
> is
> not written at all in this case.
>
> So for example if this code stores 5 correct looking mmcos and the
> 6th
> is
> invalid, 6 are in the array but the number of entries is just left
> where
> it
> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the
> invalid
> value
> later doesnt feel ideal.

 Nope, mmco state is left in inconsistent state, and my patch fix it.
 As
 you
 provided nothing valuable to consider as alternative I will apply it.

>>>
>>> What about converting any invalid mmco opcode to mmco reset and
>>> updating mmco size too?
>>> Currently mmco size is left in previous state.
>>
>> Don't invent a new RESET (= 5) operation - that type is special and its
>> presence implies other constraints which we probably don't want to
>> think
>> about here (around frame_num in particular).
>>
>> I think END / truncating the list would be best option?
>>
>
> Nope, that would still put it into bad state. With your approach decoder
> does
> not recover from artifacts. Try sample from bug report #7374.

 Adding a spurious reset here throws away all previous reference frames,
 which will break the stream where it wouldn't otherwise be if the
 corrupted frame would have been bypassed for referencing.  For example,
 in
 real-time cases with feedback a stream which has encountered errors can
 be
 recovered without an intra frame by referring to an older frame which
 still exists in the DPB.
>>>
>>> Sample: .  The bad frame
>>> here has frame_num 24, but we already hit an error before that point and
>>> the
>>> feedback about that makes frame_num 25 refer to LTRF 1 such that 24 is
>>> never
>>> used.  (From a simulator rather than a real capture, because random bit
>>> errors are never where you want them.)
>>>
>>> It doesn't exactly hit the original issue because the leftover MMCO count
>>> from the previous slice is not large enough.  With:
>>>
>>> diff --git a/libavcodec/

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-11 Thread Paul B Mahol 
On 12/11/18, Carl Eugen Hoyos  wrote:
> 2018-12-10 23:41 GMT+01:00, Mark Thompson :
>> On 09/12/2018 14:21, Mark Thompson wrote:
>>> On 09/12/2018 13:54, Paul B Mahol wrote:
 On 12/9/18, Mark Thompson  wrote:
> On 09/12/2018 08:52, Paul B Mahol wrote:
>> On 12/7/18, Paul B Mahol  wrote:
>>> On 12/7/18, Michael Niedermayer  wrote:
 On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol  wrote:
>> On 12/7/18, Michael Niedermayer  wrote:
>>> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
 This recovers state with #7374 linked sample.

 Work funded by Open Broadcast Systems.

 Signed-off-by: Paul B Mahol 
 ---
  libavcodec/h264_refs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
 index eaf965e43d..5645a203a7 100644
 --- a/libavcodec/h264_refs.c
 +++ b/libavcodec/h264_refs.c
 @@ -718,6 +718,7 @@ int
 ff_h264_execute_ref_pic_marking(H264Context
 *h)
  }
  break;
  case MMCO_RESET:
 +default:
  while (h->short_ref_count) {
  remove_short(h, h->short_ref[0]->frame_num, 0);
  }
 @@ -730,7 +731,6 @@ int
 ff_h264_execute_ref_pic_marking(H264Context
 *h)
  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
  h->last_pocs[j] = INT_MIN;
  break;
 -default: assert(0);
  }
  }
>>>
>>> mmco[i].opcode should not be invalid, its checked around the
>>> point
>>> where
>>> this
>>> array is filled.
>>> unless there is something iam missing
>>
>> Yes, you are missing big time.
>> If you think by "checked" about those nice asserts they are not
>> enabled
>> at
>> all.
>>
>
> There is check for invalid opcode, but stored invalid opcode is not
> changed.

 Theres no question that we end with a invalid value in the struct,
 but
 that
 is not intended to be in there. You can see that this is not
 intended
 by
 simply looking at the variable that holds the number of entries, it
 is
 not written at all in this case.

 So for example if this code stores 5 correct looking mmcos and the
 6th
 is
 invalid, 6 are in the array but the number of entries is just left
 where
 it
 was, that could be maybe 3 or 8 or 1. Just "defaulting out" the
 invalid
 value
 later doesnt feel ideal.
>>>
>>> Nope, mmco state is left in inconsistent state, and my patch fix it.
>>> As
>>> you
>>> provided nothing valuable to consider as alternative I will apply it.
>>>
>>
>> What about converting any invalid mmco opcode to mmco reset and
>> updating mmco size too?
>> Currently mmco size is left in previous state.
>
> Don't invent a new RESET (= 5) operation - that type is special and its
> presence implies other constraints which we probably don't want to
> think
> about here (around frame_num in particular).
>
> I think END / truncating the list would be best option?
>

 Nope, that would still put it into bad state. With your approach decoder
 does
 not recover from artifacts. Try sample from bug report #7374.
>>>
>>> Adding a spurious reset here throws away all previous reference frames,
>>> which will break the stream where it wouldn't otherwise be if the
>>> corrupted frame would have been bypassed for referencing.  For example,
>>> in
>>> real-time cases with feedback a stream which has encountered errors can
>>> be
>>> recovered without an intra frame by referring to an older frame which
>>> still exists in the DPB.
>>
>> Sample: .  The bad frame
>> here has frame_num 24, but we already hit an error before that point and
>> the
>> feedback about that makes frame_num 25 refer to LTRF 1 such that 24 is
>> never
>> used.  (From a simulator rather than a real capture, because random bit
>> errors are never where you want them.)
>>
>> It doesn't exactly hit the original issue because the leftover MMCO count
>> from the previous slice is not large enough.  With:
>>
>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> index 5645a203a7..977b4ed12f 100644
>> --- a/libavcodec/h264_refs.c
>> +++ b/libavcodec/h264_refs.c
>> @@ -875,6 +875,7 @@ int f

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-10 Thread Carl Eugen Hoyos 
2018-12-10 23:41 GMT+01:00, Mark Thompson :
> On 09/12/2018 14:21, Mark Thompson wrote:
>> On 09/12/2018 13:54, Paul B Mahol wrote:
>>> On 12/9/18, Mark Thompson  wrote:
 On 09/12/2018 08:52, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol  wrote:
>> On 12/7/18, Michael Niedermayer  wrote:
>>> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
 On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>>> This recovers state with #7374 linked sample.
>>>
>>> Work funded by Open Broadcast Systems.
>>>
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavcodec/h264_refs.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>>> index eaf965e43d..5645a203a7 100644
>>> --- a/libavcodec/h264_refs.c
>>> +++ b/libavcodec/h264_refs.c
>>> @@ -718,6 +718,7 @@ int
>>> ff_h264_execute_ref_pic_marking(H264Context
>>> *h)
>>>  }
>>>  break;
>>>  case MMCO_RESET:
>>> +default:
>>>  while (h->short_ref_count) {
>>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>>  }
>>> @@ -730,7 +731,6 @@ int
>>> ff_h264_execute_ref_pic_marking(H264Context
>>> *h)
>>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>>  h->last_pocs[j] = INT_MIN;
>>>  break;
>>> -default: assert(0);
>>>  }
>>>  }
>>
>> mmco[i].opcode should not be invalid, its checked around the point
>> where
>> this
>> array is filled.
>> unless there is something iam missing
>
> Yes, you are missing big time.
> If you think by "checked" about those nice asserts they are not
> enabled
> at
> all.
>

 There is check for invalid opcode, but stored invalid opcode is not
 changed.
>>>
>>> Theres no question that we end with a invalid value in the struct,
>>> but
>>> that
>>> is not intended to be in there. You can see that this is not intended
>>> by
>>> simply looking at the variable that holds the number of entries, it
>>> is
>>> not written at all in this case.
>>>
>>> So for example if this code stores 5 correct looking mmcos and the
>>> 6th
>>> is
>>> invalid, 6 are in the array but the number of entries is just left
>>> where
>>> it
>>> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the
>>> invalid
>>> value
>>> later doesnt feel ideal.
>>
>> Nope, mmco state is left in inconsistent state, and my patch fix it.
>> As
>> you
>> provided nothing valuable to consider as alternative I will apply it.
>>
>
> What about converting any invalid mmco opcode to mmco reset and
> updating mmco size too?
> Currently mmco size is left in previous state.

 Don't invent a new RESET (= 5) operation - that type is special and its
 presence implies other constraints which we probably don't want to think
 about here (around frame_num in particular).

 I think END / truncating the list would be best option?

>>>
>>> Nope, that would still put it into bad state. With your approach decoder
>>> does
>>> not recover from artifacts. Try sample from bug report #7374.
>>
>> Adding a spurious reset here throws away all previous reference frames,
>> which will break the stream where it wouldn't otherwise be if the
>> corrupted frame would have been bypassed for referencing.  For example, in
>> real-time cases with feedback a stream which has encountered errors can be
>> recovered without an intra frame by referring to an older frame which
>> still exists in the DPB.
>
> Sample: .  The bad frame
> here has frame_num 24, but we already hit an error before that point and the
> feedback about that makes frame_num 25 refer to LTRF 1 such that 24 is never
> used.  (From a simulator rather than a real capture, because random bit
> errors are never where you want them.)
>
> It doesn't exactly hit the original issue because the leftover MMCO count
> from the previous slice is not large enough.  With:
>
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index 5645a203a7..977b4ed12f 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -875,6 +875,7 @@ int ff_h264_decode_ref_pic_marking(H264SliceContext *sl,
> GetBitContext *gb,
>  av_log(logctx, AV_LOG_ERROR,
> "illegal memory management control operation
>

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-10 Thread Mark Thompson 
On 09/12/2018 14:21, Mark Thompson wrote:
> On 09/12/2018 13:54, Paul B Mahol wrote:
>> On 12/9/18, Mark Thompson  wrote:
>>> On 09/12/2018 08:52, Paul B Mahol wrote:
 On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>>> On 12/7/18, Paul B Mahol  wrote:
 On 12/7/18, Michael Niedermayer  wrote:
> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>> This recovers state with #7374 linked sample.
>>
>> Work funded by Open Broadcast Systems.
>>
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavcodec/h264_refs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> index eaf965e43d..5645a203a7 100644
>> --- a/libavcodec/h264_refs.c
>> +++ b/libavcodec/h264_refs.c
>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
>> *h)
>>  }
>>  break;
>>  case MMCO_RESET:
>> +default:
>>  while (h->short_ref_count) {
>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>  }
>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
>> *h)
>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>  h->last_pocs[j] = INT_MIN;
>>  break;
>> -default: assert(0);
>>  }
>>  }
>
> mmco[i].opcode should not be invalid, its checked around the point
> where
> this
> array is filled.
> unless there is something iam missing

 Yes, you are missing big time.
 If you think by "checked" about those nice asserts they are not
 enabled
 at
 all.

>>>
>>> There is check for invalid opcode, but stored invalid opcode is not
>>> changed.
>>
>> Theres no question that we end with a invalid value in the struct, but
>> that
>> is not intended to be in there. You can see that this is not intended by
>> simply looking at the variable that holds the number of entries, it is
>> not written at all in this case.
>>
>> So for example if this code stores 5 correct looking mmcos and the 6th
>> is
>> invalid, 6 are in the array but the number of entries is just left where
>> it
>> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
>> value
>> later doesnt feel ideal.
>
> Nope, mmco state is left in inconsistent state, and my patch fix it. As
> you
> provided nothing valuable to consider as alternative I will apply it.
>

 What about converting any invalid mmco opcode to mmco reset and
 updating mmco size too?
 Currently mmco size is left in previous state.
>>>
>>> Don't invent a new RESET (= 5) operation - that type is special and its
>>> presence implies other constraints which we probably don't want to think
>>> about here (around frame_num in particular).
>>>
>>> I think END / truncating the list would be best option?
>>>
>>
>> Nope, that would still put it into bad state. With your approach decoder does
>> not recover from artifacts. Try sample from bug report #7374.
> 
> Adding a spurious reset here throws away all previous reference frames, which 
> will break the stream where it wouldn't otherwise be if the corrupted frame 
> would have been bypassed for referencing.  For example, in real-time cases 
> with feedback a stream which has encountered errors can be recovered without 
> an intra frame by referring to an older frame which still exists in the DPB.

Sample: .  The bad frame here 
has frame_num 24, but we already hit an error before that point and the 
feedback about that makes frame_num 25 refer to LTRF 1 such that 24 is never 
used.  (From a simulator rather than a real capture, because random bit errors 
are never where you want them.)

It doesn't exactly hit the original issue because the leftover MMCO count from 
the previous slice is not large enough.  With:

diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
index 5645a203a7..977b4ed12f 100644
--- a/libavcodec/h264_refs.c
+++ b/libavcodec/h264_refs.c
@@ -875,6 +875,7 @@ int ff_h264_decode_ref_pic_marking(H264SliceContext *sl, 
GetBitContext *gb,
 av_log(logctx, AV_LOG_ERROR,
"illegal memory management control operation %d\n",
opcode);
+sl->nb_mmco = i + 1;
 return -1;
 }
 if (opcode == MMCO_END)

to make sure the MMCO count is written with a high enough value it does

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-09 Thread Paul B Mahol 
On 12/9/18, Mark Thompson  wrote:
> On 09/12/2018 13:54, Paul B Mahol wrote:
>> On 12/9/18, Mark Thompson  wrote:
>>> On 09/12/2018 08:52, Paul B Mahol wrote:
 On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>>> On 12/7/18, Paul B Mahol  wrote:
 On 12/7/18, Michael Niedermayer  wrote:
> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>> This recovers state with #7374 linked sample.
>>
>> Work funded by Open Broadcast Systems.
>>
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavcodec/h264_refs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> index eaf965e43d..5645a203a7 100644
>> --- a/libavcodec/h264_refs.c
>> +++ b/libavcodec/h264_refs.c
>> @@ -718,6 +718,7 @@ int
>> ff_h264_execute_ref_pic_marking(H264Context
>> *h)
>>  }
>>  break;
>>  case MMCO_RESET:
>> +default:
>>  while (h->short_ref_count) {
>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>  }
>> @@ -730,7 +731,6 @@ int
>> ff_h264_execute_ref_pic_marking(H264Context
>> *h)
>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>  h->last_pocs[j] = INT_MIN;
>>  break;
>> -default: assert(0);
>>  }
>>  }
>
> mmco[i].opcode should not be invalid, its checked around the point
> where
> this
> array is filled.
> unless there is something iam missing

 Yes, you are missing big time.
 If you think by "checked" about those nice asserts they are not
 enabled
 at
 all.

>>>
>>> There is check for invalid opcode, but stored invalid opcode is not
>>> changed.
>>
>> Theres no question that we end with a invalid value in the struct, but
>> that
>> is not intended to be in there. You can see that this is not intended
>> by
>> simply looking at the variable that holds the number of entries, it is
>> not written at all in this case.
>>
>> So for example if this code stores 5 correct looking mmcos and the 6th
>> is
>> invalid, 6 are in the array but the number of entries is just left
>> where
>> it
>> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the
>> invalid
>> value
>> later doesnt feel ideal.
>
> Nope, mmco state is left in inconsistent state, and my patch fix it. As
> you
> provided nothing valuable to consider as alternative I will apply it.
>

 What about converting any invalid mmco opcode to mmco reset and
 updating mmco size too?
 Currently mmco size is left in previous state.
>>>
>>> Don't invent a new RESET (= 5) operation - that type is special and its
>>> presence implies other constraints which we probably don't want to think
>>> about here (around frame_num in particular).
>>>
>>> I think END / truncating the list would be best option?
>>>
>>
>> Nope, that would still put it into bad state. With your approach decoder
>> does
>> not recover from artifacts. Try sample from bug report #7374.
>
> Adding a spurious reset here throws away all previous reference frames,
> which will break the stream where it wouldn't otherwise be if the corrupted
> frame would have been bypassed for referencing.  For example, in real-time
> cases with feedback a stream which has encountered errors can be recovered
> without an intra frame by referring to an older frame which still exists in
> the DPB.

So instead of fixing all frames after error you prefer keeping old
code which will keep spurious errors all the time, keeping decoder
useless.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-09 Thread Mark Thompson 
On 09/12/2018 13:54, Paul B Mahol wrote:
> On 12/9/18, Mark Thompson  wrote:
>> On 09/12/2018 08:52, Paul B Mahol wrote:
>>> On 12/7/18, Paul B Mahol  wrote:
 On 12/7/18, Michael Niedermayer  wrote:
> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>> On 12/7/18, Paul B Mahol  wrote:
>>> On 12/7/18, Michael Niedermayer  wrote:
 On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
> This recovers state with #7374 linked sample.
>
> Work funded by Open Broadcast Systems.
>
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/h264_refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index eaf965e43d..5645a203a7 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
> *h)
>  }
>  break;
>  case MMCO_RESET:
> +default:
>  while (h->short_ref_count) {
>  remove_short(h, h->short_ref[0]->frame_num, 0);
>  }
> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
> *h)
>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>  h->last_pocs[j] = INT_MIN;
>  break;
> -default: assert(0);
>  }
>  }

 mmco[i].opcode should not be invalid, its checked around the point
 where
 this
 array is filled.
 unless there is something iam missing
>>>
>>> Yes, you are missing big time.
>>> If you think by "checked" about those nice asserts they are not
>>> enabled
>>> at
>>> all.
>>>
>>
>> There is check for invalid opcode, but stored invalid opcode is not
>> changed.
>
> Theres no question that we end with a invalid value in the struct, but
> that
> is not intended to be in there. You can see that this is not intended by
> simply looking at the variable that holds the number of entries, it is
> not written at all in this case.
>
> So for example if this code stores 5 correct looking mmcos and the 6th
> is
> invalid, 6 are in the array but the number of entries is just left where
> it
> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
> value
> later doesnt feel ideal.

 Nope, mmco state is left in inconsistent state, and my patch fix it. As
 you
 provided nothing valuable to consider as alternative I will apply it.

>>>
>>> What about converting any invalid mmco opcode to mmco reset and
>>> updating mmco size too?
>>> Currently mmco size is left in previous state.
>>
>> Don't invent a new RESET (= 5) operation - that type is special and its
>> presence implies other constraints which we probably don't want to think
>> about here (around frame_num in particular).
>>
>> I think END / truncating the list would be best option?
>>
> 
> Nope, that would still put it into bad state. With your approach decoder does
> not recover from artifacts. Try sample from bug report #7374.

Adding a spurious reset here throws away all previous reference frames, which 
will break the stream where it wouldn't otherwise be if the corrupted frame 
would have been bypassed for referencing.  For example, in real-time cases with 
feedback a stream which has encountered errors can be recovered without an 
intra frame by referring to an older frame which still exists in the DPB.

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

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-09 Thread Paul B Mahol 
On 12/9/18, Mark Thompson  wrote:
> On 09/12/2018 08:52, Paul B Mahol wrote:
>> On 12/7/18, Paul B Mahol  wrote:
>>> On 12/7/18, Michael Niedermayer  wrote:
 On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol  wrote:
>> On 12/7/18, Michael Niedermayer  wrote:
>>> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
 This recovers state with #7374 linked sample.

 Work funded by Open Broadcast Systems.

 Signed-off-by: Paul B Mahol 
 ---
  libavcodec/h264_refs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
 index eaf965e43d..5645a203a7 100644
 --- a/libavcodec/h264_refs.c
 +++ b/libavcodec/h264_refs.c
 @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
 *h)
  }
  break;
  case MMCO_RESET:
 +default:
  while (h->short_ref_count) {
  remove_short(h, h->short_ref[0]->frame_num, 0);
  }
 @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
 *h)
  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
  h->last_pocs[j] = INT_MIN;
  break;
 -default: assert(0);
  }
  }
>>>
>>> mmco[i].opcode should not be invalid, its checked around the point
>>> where
>>> this
>>> array is filled.
>>> unless there is something iam missing
>>
>> Yes, you are missing big time.
>> If you think by "checked" about those nice asserts they are not
>> enabled
>> at
>> all.
>>
>
> There is check for invalid opcode, but stored invalid opcode is not
> changed.

 Theres no question that we end with a invalid value in the struct, but
 that
 is not intended to be in there. You can see that this is not intended by
 simply looking at the variable that holds the number of entries, it is
 not written at all in this case.

 So for example if this code stores 5 correct looking mmcos and the 6th
 is
 invalid, 6 are in the array but the number of entries is just left where
 it
 was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
 value
 later doesnt feel ideal.
>>>
>>> Nope, mmco state is left in inconsistent state, and my patch fix it. As
>>> you
>>> provided nothing valuable to consider as alternative I will apply it.
>>>
>>
>> What about converting any invalid mmco opcode to mmco reset and
>> updating mmco size too?
>> Currently mmco size is left in previous state.
>
> Don't invent a new RESET (= 5) operation - that type is special and its
> presence implies other constraints which we probably don't want to think
> about here (around frame_num in particular).
>
> I think END / truncating the list would be best option?
>

Nope, that would still put it into bad state. With your approach decoder does
not recover from artifacts. Try sample from bug report #7374.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-09 Thread Mark Thompson 
On 09/12/2018 08:52, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol  wrote:
>> On 12/7/18, Michael Niedermayer  wrote:
>>> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
 On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>>> This recovers state with #7374 linked sample.
>>>
>>> Work funded by Open Broadcast Systems.
>>>
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavcodec/h264_refs.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>>> index eaf965e43d..5645a203a7 100644
>>> --- a/libavcodec/h264_refs.c
>>> +++ b/libavcodec/h264_refs.c
>>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
>>> *h)
>>>  }
>>>  break;
>>>  case MMCO_RESET:
>>> +default:
>>>  while (h->short_ref_count) {
>>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>>  }
>>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
>>> *h)
>>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>>  h->last_pocs[j] = INT_MIN;
>>>  break;
>>> -default: assert(0);
>>>  }
>>>  }
>>
>> mmco[i].opcode should not be invalid, its checked around the point
>> where
>> this
>> array is filled.
>> unless there is something iam missing
>
> Yes, you are missing big time.
> If you think by "checked" about those nice asserts they are not
> enabled
> at
> all.
>

 There is check for invalid opcode, but stored invalid opcode is not
 changed.
>>>
>>> Theres no question that we end with a invalid value in the struct, but
>>> that
>>> is not intended to be in there. You can see that this is not intended by
>>> simply looking at the variable that holds the number of entries, it is
>>> not written at all in this case.
>>>
>>> So for example if this code stores 5 correct looking mmcos and the 6th is
>>> invalid, 6 are in the array but the number of entries is just left where
>>> it
>>> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
>>> value
>>> later doesnt feel ideal.
>>
>> Nope, mmco state is left in inconsistent state, and my patch fix it. As you
>> provided nothing valuable to consider as alternative I will apply it.
>>
> 
> What about converting any invalid mmco opcode to mmco reset and
> updating mmco size too?
> Currently mmco size is left in previous state.

Don't invent a new RESET (= 5) operation - that type is special and its 
presence implies other constraints which we probably don't want to think about 
here (around frame_num in particular).

I think END / truncating the list would be best option?

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

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-09 Thread Paul B Mahol 
On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>>> On 12/7/18, Paul B Mahol  wrote:
>>> > On 12/7/18, Michael Niedermayer  wrote:
>>> >> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>>> >>> This recovers state with #7374 linked sample.
>>> >>>
>>> >>> Work funded by Open Broadcast Systems.
>>> >>>
>>> >>> Signed-off-by: Paul B Mahol 
>>> >>> ---
>>> >>>  libavcodec/h264_refs.c | 2 +-
>>> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>>
>>> >>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>>> >>> index eaf965e43d..5645a203a7 100644
>>> >>> --- a/libavcodec/h264_refs.c
>>> >>> +++ b/libavcodec/h264_refs.c
>>> >>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
>>> >>> *h)
>>> >>>  }
>>> >>>  break;
>>> >>>  case MMCO_RESET:
>>> >>> +default:
>>> >>>  while (h->short_ref_count) {
>>> >>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>> >>>  }
>>> >>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
>>> >>> *h)
>>> >>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>> >>>  h->last_pocs[j] = INT_MIN;
>>> >>>  break;
>>> >>> -default: assert(0);
>>> >>>  }
>>> >>>  }
>>> >>
>>> >> mmco[i].opcode should not be invalid, its checked around the point
>>> >> where
>>> >> this
>>> >> array is filled.
>>> >> unless there is something iam missing
>>> >
>>> > Yes, you are missing big time.
>>> > If you think by "checked" about those nice asserts they are not
>>> > enabled
>>> > at
>>> > all.
>>> >
>>>
>>> There is check for invalid opcode, but stored invalid opcode is not
>>> changed.
>>
>> Theres no question that we end with a invalid value in the struct, but
>> that
>> is not intended to be in there. You can see that this is not intended by
>> simply looking at the variable that holds the number of entries, it is
>> not written at all in this case.
>>
>> So for example if this code stores 5 correct looking mmcos and the 6th is
>> invalid, 6 are in the array but the number of entries is just left where
>> it
>> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
>> value
>> later doesnt feel ideal.
>
> Nope, mmco state is left in inconsistent state, and my patch fix it. As you
> provided nothing valuable to consider as alternative I will apply it.
>

What about converting any invalid mmco opcode to mmco reset and
updating mmco size too?
Currently mmco size is left in previous state.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Paul B Mahol 
On 12/7/18, Michael Niedermayer  wrote:
> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>> On 12/7/18, Paul B Mahol  wrote:
>> > On 12/7/18, Michael Niedermayer  wrote:
>> >> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>> >>> This recovers state with #7374 linked sample.
>> >>>
>> >>> Work funded by Open Broadcast Systems.
>> >>>
>> >>> Signed-off-by: Paul B Mahol 
>> >>> ---
>> >>>  libavcodec/h264_refs.c | 2 +-
>> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> >>> index eaf965e43d..5645a203a7 100644
>> >>> --- a/libavcodec/h264_refs.c
>> >>> +++ b/libavcodec/h264_refs.c
>> >>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
>> >>> *h)
>> >>>  }
>> >>>  break;
>> >>>  case MMCO_RESET:
>> >>> +default:
>> >>>  while (h->short_ref_count) {
>> >>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>> >>>  }
>> >>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
>> >>> *h)
>> >>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>> >>>  h->last_pocs[j] = INT_MIN;
>> >>>  break;
>> >>> -default: assert(0);
>> >>>  }
>> >>>  }
>> >>
>> >> mmco[i].opcode should not be invalid, its checked around the point
>> >> where
>> >> this
>> >> array is filled.
>> >> unless there is something iam missing
>> >
>> > Yes, you are missing big time.
>> > If you think by "checked" about those nice asserts they are not enabled
>> > at
>> > all.
>> >
>>
>> There is check for invalid opcode, but stored invalid opcode is not
>> changed.
>
> Theres no question that we end with a invalid value in the struct, but that
> is not intended to be in there. You can see that this is not intended by
> simply looking at the variable that holds the number of entries, it is
> not written at all in this case.
>
> So for example if this code stores 5 correct looking mmcos and the 6th is
> invalid, 6 are in the array but the number of entries is just left where it
> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
> value
> later doesnt feel ideal.

Nope, mmco state is left in inconsistent state, and my patch fix it. As you
provided nothing valuable to consider as alternative I will apply it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Michael Niedermayer 
On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol  wrote:
> > On 12/7/18, Michael Niedermayer  wrote:
> >> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
> >>> This recovers state with #7374 linked sample.
> >>>
> >>> Work funded by Open Broadcast Systems.
> >>>
> >>> Signed-off-by: Paul B Mahol 
> >>> ---
> >>>  libavcodec/h264_refs.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> >>> index eaf965e43d..5645a203a7 100644
> >>> --- a/libavcodec/h264_refs.c
> >>> +++ b/libavcodec/h264_refs.c
> >>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
> >>>  }
> >>>  break;
> >>>  case MMCO_RESET:
> >>> +default:
> >>>  while (h->short_ref_count) {
> >>>  remove_short(h, h->short_ref[0]->frame_num, 0);
> >>>  }
> >>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
> >>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
> >>>  h->last_pocs[j] = INT_MIN;
> >>>  break;
> >>> -default: assert(0);
> >>>  }
> >>>  }
> >>
> >> mmco[i].opcode should not be invalid, its checked around the point where
> >> this
> >> array is filled.
> >> unless there is something iam missing
> >
> > Yes, you are missing big time.
> > If you think by "checked" about those nice asserts they are not enabled at
> > all.
> >
> 
> There is check for invalid opcode, but stored invalid opcode is not changed.

Theres no question that we end with a invalid value in the struct, but that
is not intended to be in there. You can see that this is not intended by
simply looking at the variable that holds the number of entries, it is
not written at all in this case.

So for example if this code stores 5 correct looking mmcos and the 6th is
invalid, 6 are in the array but the number of entries is just left where it
was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid value
later doesnt feel ideal.


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

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


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

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Carl Eugen Hoyos 
2018-12-06 15:26 GMT+01:00, Paul B Mahol :
> This recovers state with #7374 linked sample.
>
> Work funded by Open Broadcast Systems.
>
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/h264_refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index eaf965e43d..5645a203a7 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>  }
>  break;
>  case MMCO_RESET:
> +default:
>  while (h->short_ref_count) {
>  remove_short(h, h->short_ref[0]->frame_num, 0);
>  }
> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>  h->last_pocs[j] = INT_MIN;
>  break;
> -default: assert(0);
>  }
>  }
>

Alternative is to avoid an invalid mmco opcode.

Carl Eugen

diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
index eaf965e..c7e64ec 100644
--- a/libavcodec/h264_refs.c
+++ b/libavcodec/h264_refs.c
@@ -875,6 +875,7 @@
 av_log(logctx, AV_LOG_ERROR,
"illegal memory management control operation %d\n",
opcode);
+mmco[i].opcode = MMCO_RESET;
 return -1;
 }
 if (opcode == MMCO_END)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Paul B Mahol 
On 12/7/18, Michael Niedermayer  wrote:
> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>> This recovers state with #7374 linked sample.
>>
>> Work funded by Open Broadcast Systems.
>>
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavcodec/h264_refs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> index eaf965e43d..5645a203a7 100644
>> --- a/libavcodec/h264_refs.c
>> +++ b/libavcodec/h264_refs.c
>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>  }
>>  break;
>>  case MMCO_RESET:
>> +default:
>>  while (h->short_ref_count) {
>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>  }
>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>  h->last_pocs[j] = INT_MIN;
>>  break;
>> -default: assert(0);
>>  }
>>  }
>
> mmco[i].opcode should not be invalid, its checked around the point where
> this
> array is filled.
> unless there is something iam missing

Yes, you are missing big time.
If you think by "checked" about those nice asserts they are not enabled at all.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Paul B Mahol 
On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>>> This recovers state with #7374 linked sample.
>>>
>>> Work funded by Open Broadcast Systems.
>>>
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavcodec/h264_refs.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>>> index eaf965e43d..5645a203a7 100644
>>> --- a/libavcodec/h264_refs.c
>>> +++ b/libavcodec/h264_refs.c
>>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>>  }
>>>  break;
>>>  case MMCO_RESET:
>>> +default:
>>>  while (h->short_ref_count) {
>>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>>  }
>>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>>  h->last_pocs[j] = INT_MIN;
>>>  break;
>>> -default: assert(0);
>>>  }
>>>  }
>>
>> mmco[i].opcode should not be invalid, its checked around the point where
>> this
>> array is filled.
>> unless there is something iam missing
>
> Yes, you are missing big time.
> If you think by "checked" about those nice asserts they are not enabled at
> all.
>

There is check for invalid opcode, but stored invalid opcode is not changed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-06 Thread Michael Niedermayer 
On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
> This recovers state with #7374 linked sample.
> 
> Work funded by Open Broadcast Systems.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/h264_refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index eaf965e43d..5645a203a7 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>  }
>  break;
>  case MMCO_RESET:
> +default:
>  while (h->short_ref_count) {
>  remove_short(h, h->short_ref[0]->frame_num, 0);
>  }
> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>  h->last_pocs[j] = INT_MIN;
>  break;
> -default: assert(0);
>  }
>  }

mmco[i].opcode should not be invalid, its checked around the point where this
array is filled. 
unless there is something iam missing

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


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

[FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-06 Thread Paul B Mahol 
This recovers state with #7374 linked sample.

Work funded by Open Broadcast Systems.

Signed-off-by: Paul B Mahol 
---
 libavcodec/h264_refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
index eaf965e43d..5645a203a7 100644
--- a/libavcodec/h264_refs.c
+++ b/libavcodec/h264_refs.c
@@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
 }
 break;
 case MMCO_RESET:
+default:
 while (h->short_ref_count) {
 remove_short(h, h->short_ref[0]->frame_num, 0);
 }
@@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
 for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
 h->last_pocs[j] = INT_MIN;
 break;
-default: assert(0);
 }
 }
 
-- 
2.17.1

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