commit d487c9cd78d68f080d275c09114e0b3ffc6a68d8
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Fri Apr 13 22:04:38 2012 +0200

    fix crash
    
    last_sc is not reset every time a frame is being output, which can cause
    last_sc > buf_size in subsequent frame.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=674073

Could you please make sure to prefix the commit message with the element or plugin name in future? :)

Sure

Could you also explain (with whatever sample prompting this?) why/how come that last_sc is not being reset?

Baseclass should be passing _NEW_FRAME flag for any new/subsequent frame, which triggers a frame reset which in turn clears the last_sc state.

In summary, afaik, any subsequent frame should always have a reset last_sc, unless something going wrong at baseclass level which should then be fixed.  There is lot more parser state handling depending on that than a single field here.

I can revert the fix locally and try to reproduce it again.

So, I've reverted the patch and tested with the file I think caused the crash and it doesn't happen anymore. It might have been caused by the parser not splitting frames properly (part 3 of #674113). I think perhaps the fix should be reverted and if I encounter any more crashes I'll look into it.

Created attachment 214565 [details] [review]
Revert previous patch

OK, makes sense, so reverted with commit below.

commit f6942d851ca295681eea03e309b800cbbceb5327
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Mon May 21 16:37:37 2012 +0200

    mpegvideoparse: Revert "fix crash"
    
    This reverts commit 91210831ee672343a296f31357144359d5c2e768.
    
    Such explicit reset should not be needed as it is arranged for by
    the baseclass in unison with monitoring for a new frame by subclass.
    
    As such it might wrongfully hide something else going on ...
    
    See #674073.