[FFmpeg-devel] MJPEG bug
Michael Niedermayer
michaelni at gmx.at
Fri Oct 7 16:48:50 CEST 2011
On Thu, Oct 06, 2011 at 08:25:04PM -0700, Aaron Miller wrote:
> Michael,
>
> This appears to fix it. I did not get any errors when running it on the
> problematic file I've been working with, but I have yet to test it on other
> files. The output plays fine.
>
> This uses the state64 field of the ParseContext structure in a non-standard
> way, that invalidates the comment in the header file: it stores two 16 bit
> variables in the low 32 bits of state64.
Please add a MjpegParser context with 2 seperate variables
>
> Cheers,
> Aaron
>
> On Mon, Oct 3, 2011 at 9:33 AM, Michael Niedermayer <michaelni at gmx.at>wrote:
>
> > On Sun, Oct 02, 2011 at 11:56:44PM -0700, Aaron Miller wrote:
> > > Michael,
> > >
> > > Here is one problem frame from the (large) raw MJPEG stream I have.
> > >
> > > And thanks for the tip about mjpeg_parser.c! I believe that's what I need
> > to
> > > change.
> >
> > ok, if you manage to solve it please send a patch, if not please tell
> > me and ill fix it
> >
> > [...]
> >
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Freedom in capitalist society always remains about the same as it was in
> > ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> mjpeg_parser.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 51 insertions(+), 8 deletions(-)
> a2b4a2cf894114f40780cc14338b84384232d7b3 mjpeg_parser.c.diff
> --- libavcodec_mjpeg_parser.c 2011-10-06 18:52:01.000000000 -0700
> +++ mjpeg_parser.c 2011-10-06 18:55:14.000000000 -0700
> @@ -36,18 +36,41 @@
> static int find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size){
> int vop_found, i;
> uint16_t state;
> + uint16_t bytes_to_length; // number of bytes before the 2-byte length has been read into state (above)
> + uint16_t payload_remaining; // number of bytes left in the payload
>
> vop_found= pc->frame_start_found;
> state= pc->state;
> + bytes_to_length = pc->state64 >> 16;
> + payload_remaining = pc->state64 & 0xffff;
>
> i=0;
> if(!vop_found){
> for(i=0; i<buf_size; i++){
> state= (state<<8) | buf[i];
> - if(state == 0xFFD8){
> - i++;
> - vop_found=1;
> - break;
> + if (bytes_to_length) {
> + if (bytes_to_length == 1) {
> + bytes_to_length = 0;
> + payload_remaining = state - 1; // 2-byte length includes itself (but not the marker just passed)
> + } else {
> + bytes_to_length--;
> + }
> + } else {
> + if (payload_remaining) {
> + payload_remaining--;
> + continue;
running this complicated loop per byte is quite slow. The payload could
in many cases just be skiped at least partially
> + }
> + if ((0xffc0 <= state) && (state <= 0xfffe)) {
> + if(state == 0xFFD8){
> + i++;
> + vop_found=1;
> + pc->state64=0; // not sure if necessary
> + break;
> + } else if ((state <= 0xffcf) || (state >= 0xffda)) {
> + // has 2-byte length and payload
> + bytes_to_length = 2;
> + }
> + }
> }
> }
> }
> @@ -58,15 +81,35 @@
> return 0;
> for(; i<buf_size; i++){
> state= (state<<8) | buf[i];
> - if(state == 0xFFD8){
> - pc->frame_start_found=0;
> - pc->state=0;
> - return i-1;
> + if (bytes_to_length) {
> + if (bytes_to_length == 1) {
> + bytes_to_length = 0;
> + payload_remaining = state - 1; // 2-byte length includes itself (but not the marker just passed)
> + } else {
> + bytes_to_length--;
> + }
> + } else {
> + if (payload_remaining) {
> + payload_remaining--;
> + continue;
> + }
> + if ((0xffc0 <= state) && (state <= 0xfffe)) {
> + if(state == 0xFFD8){
> + pc->frame_start_found=0;
> + pc->state=0;
> + pc->state64=0; // bytes_to_length and payload_remaining
> + return i-1;
> + } else if ((state <= 0xffcf) || (state >= 0xffda)) {
> + // has 2-byte length and payload
> + bytes_to_length = 2;
> + }
> + }
> }
> }
> }
> pc->frame_start_found= vop_found;
> pc->state= state;
> + pc->state64 = (bytes_to_length << 16) | payload_remaining;
> return END_NOT_FOUND;
> }
>
> mjpegdec.new.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
> 5fdd950e91159e7ee0f464155504fd31805a8b51 mjpegdec.c.diff
> --- /Users/atlas/mjpegdec.old.c 2011-10-06 19:42:05.000000000 -0700
> +++ /Users/atlas/mjpegdec.new.c 2011-10-06 20:02:29.000000000 -0700
> @@ -1271,7 +1271,8 @@
>
> /* return the 8 bit start code value and update the search
> state. Return -1 if no start code found */
> -static int find_marker(const uint8_t **pbuf_ptr, const uint8_t *buf_end)
> +static int find_marker(const uint8_t **pbuf_ptr, const uint8_t *buf_end,
> + const uint8_t **payload_end)
> {
> const uint8_t *buf_ptr;
> unsigned int v, v2;
> @@ -1281,6 +1282,10 @@
> #endif
>
> buf_ptr = *pbuf_ptr;
> + if (*payload_end && *payload_end > buf_ptr) {
> + buf_ptr = *payload_end;
> + }
> + *payload_end = 0;
> while (buf_ptr < buf_end) {
> v = *buf_ptr++;
> v2 = *buf_ptr;
> @@ -1301,10 +1306,12 @@
>
> int ff_mjpeg_find_marker(MJpegDecodeContext *s,
> const uint8_t **buf_ptr, const uint8_t *buf_end,
> - const uint8_t **unescaped_buf_ptr, int *unescaped_buf_size)
> + const uint8_t **unescaped_buf_ptr, int *unescaped_buf_size,
> + const uint8_t **payload_end) /* AARON addition 2011-09-30 */
git log and blame keep track of who added what and when theres no need
to use comments for that
also please generate patches with git format-patch or equivalent
> {
> int start_code;
> - start_code = find_marker(buf_ptr, buf_end);
> + unsigned int payload_size;
> + start_code = find_marker(buf_ptr, buf_end, payload_end);
>
> if ((buf_end - *buf_ptr) > s->buffer_size)
> {
> @@ -1315,6 +1322,14 @@
> s->buffer_size);
> }
>
> + if (start_code >= 0xd0 && start_code <= 0xd9) {
> + payload_size = 0;
> + } else {
> + /* variable size */
> + payload_size = ((**buf_ptr)<<8) | *(*buf_ptr + 1);
see AV_RB16()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Avoid a single point of failure, be that a person or equipment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111007/8eed459f/attachment.asc>
More information about the ffmpeg-devel
mailing list