On 09/12/2018 08:52, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol <one...@gmail.com> wrote:
>> On 12/7/18, Michael Niedermayer <mich...@niedermayer.cc> wrote:
>>> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>>>> On 12/7/18, Paul B Mahol <one...@gmail.com> wrote:
>>>>> On 12/7/18, Michael Niedermayer <mich...@niedermayer.cc> 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 <one...@gmail.com>
>>>>>>> ---
>>>>>>> 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
