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

[Michael Niedermayer]

On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol <onemda at gmail.com> wrote:
> > On 12/7/18, Michael Niedermayer <michael at 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 <onemda at 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.


[...]
-- 
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181207/606f92ad/attachment.sig>