Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image/jpeg: Unable to decode concatenated JPEGs (MIME-less "MJPEG") #22170

Open
hnnsgstfssn opened this issue Oct 6, 2017 · 10 comments
Open

image/jpeg: Unable to decode concatenated JPEGs (MIME-less "MJPEG") #22170

hnnsgstfssn opened this issue Oct 6, 2017 · 10 comments

Comments

@hnnsgstfssn
Copy link

@hnnsgstfssn hnnsgstfssn commented Oct 6, 2017

What did you do?

Wrapping ffmpeg -i <url> -c:v mjpeg -f image2pipe - in a exec.Cmd and continuously decoding the command output in a loop with jpeg.Decode fails while doing the same thing with ffmpeg -i <url> -c:v png -f image2pipe - and png.Decode works.

The following example illustrates the error by creating an MJPEG stream using a sample image: https://play.golang.org/p/lppyHZftWA

The same program but using a PNG stream and png.Decode shows successful decoding and a clean exit: https://play.golang.org/p/c54-hc6JRK

The following is a test that is expected to pass:

package test

import (
	"bytes"
	"image/jpeg"
	"image/png"
	"io"
	"testing"
)

// pngFrame is a PNG produced with
// `convert -size 1x1 xc:white png:- | xxd -c 1 -ps | sed -e 's/^/\\x/' |tr -d '\n'`
const pngFrame = "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52\x00\x00\x00\x01\x00\x00\x00\x01\x01\x00\x00\x00\x00\x37\x6e\xf9\x24\x00\x00\x00\x04\x67\x41\x4d\x41\x00\x00\xb1\x8f\x0b\xfc\x61\x05\x00\x00\x00\x20\x63\x48\x52\x4d\x00\x00\x7a\x26\x00\x00\x80\x84\x00\x00\xfa\x00\x00\x00\x80\xe8\x00\x00\x75\x30\x00\x00\xea\x60\x00\x00\x3a\x98\x00\x00\x17\x70\x9c\xba\x51\x3c\x00\x00\x00\x02\x62\x4b\x47\x44\x00\x01\xdd\x8a\x13\xa4\x00\x00\x00\x07\x74\x49\x4d\x45\x07\xe1\x0a\x06\x15\x06\x25\x79\xa4\x37\xa0\x00\x00\x00\x0a\x49\x44\x41\x54\x08\xd7\x63\x68\x00\x00\x00\x82\x00\x81\xdd\x43\x6a\xf4\x00\x00\x00\x25\x74\x45\x58\x74\x64\x61\x74\x65\x3a\x63\x72\x65\x61\x74\x65\x00\x32\x30\x31\x37\x2d\x31\x30\x2d\x30\x36\x54\x32\x31\x3a\x30\x36\x3a\x33\x37\x2b\x30\x31\x3a\x30\x30\xeb\x24\x00\x4a\x00\x00\x00\x25\x74\x45\x58\x74\x64\x61\x74\x65\x3a\x6d\x6f\x64\x69\x66\x79\x00\x32\x30\x31\x37\x2d\x31\x30\x2d\x30\x36\x54\x32\x31\x3a\x30\x36\x3a\x33\x37\x2b\x30\x31\x3a\x30\x30\x9a\x79\xb8\xf6\x00\x00\x00\x00\x49\x45\x4e\x44\xae\x42\x60\x82"

// jpegFrame is a JPEG frame produced with
// `convert -size 1x1 xc:white jpg:- | xxd -c 1 -ps | sed -e 's/^/\\x/' |tr -d '\n'`
const jpegFrame = "\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xdb\x00\x43\x00\x03\x02\x02\x02\x02\x02\x03\x02\x02\x02\x03\x03\x03\x03\x04\x06\x04\x04\x04\x04\x04\x08\x06\x06\x05\x06\x09\x08\x0a\x0a\x09\x08\x09\x09\x0a\x0c\x0f\x0c\x0a\x0b\x0e\x0b\x09\x09\x0d\x11\x0d\x0e\x0f\x10\x10\x11\x10\x0a\x0c\x12\x13\x12\x10\x13\x0f\x10\x10\x10\xff\xc0\x00\x0b\x08\x00\x01\x00\x01\x01\x01\x11\x00\xff\xc4\x00\x14\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x09\xff\xc4\x00\x14\x10\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xff\xda\x00\x08\x01\x01\x00\x00\x3f\x00\x54\xdf\xff\xd9"

func TestMultiPngDecode(t *testing.T) {

	var expected int = 3 // expected success count
	var got int          // decoded frame count
	var no int           // frame number

	stream := bytes.NewBuffer(bytes.Repeat([]byte(pngFrame), expected))

	for no = 1; no <= expected; no++ {
		_, err := png.Decode(stream)
		if err != nil {
			if err != io.EOF {
				t.Errorf("Unexpected error when decoding frame #%d: %s", no, err)
			}
			break
		}
		got++
	}

	if expected != got {
		t.Errorf("Expected %d decoded frames, got %d", expected, got)
	}
}

func TestMjpegDecode(t *testing.T) {

	var expected int = 3 // expected success count
	var got int          // decoded frame count
	var no int           // frame number

	stream := bytes.NewBuffer(bytes.Repeat([]byte(jpegFrame), expected))

	for no = 1; no <= expected; no++ {
		_, err := jpeg.Decode(stream)
		if err != nil {
			if err != io.EOF {
				t.Errorf("Unexpected error when decoding frame #%d: %s", no, err)
			}
			break
		}
		got++
	}

	if expected != got {
		t.Errorf("Expected %d decoded frames, got %d", expected, got)
	}
}

Grabbing a number of frames from a video and inspecting the output we can verify that there's no unexpected bytes after the EOI marker (ff d9) and before the next SOI marker (ff d8) and that we get the expected number of frames:

ffmpeg -v quiet -y -i <video> -frames:v 3 -c:v mjpeg -f image2pipe - | hexdump | grep "ff d8"
0000000 ff d8 ff e0 00 10 4a 46 49 46 00 01 02 00 00 01
00054d0 14 9e 80 7f ff d9 ff d8 ff e0 00 10 4a 46 49 46
000ca60 ad d0 2b 8a 29 69 29 68 7a 05 cf ff d9 ff d8 ff

ffmpeg -v quiet -y -i <video> -frames:v 3 -c:v mjpeg -f image2pipe - | hexdump | grep "ff d9"
00054d0 14 9e 80 7f ff d9 ff d8 ff e0 00 10 4a 46 49 46
000ca60 ad d0 2b 8a 29 69 29 68 7a 05 cf ff d9 ff d8 ff
0014180 ff d9

What did you expect to see?

The MJPEG stream is a concatenation of JPEG images and the decoder should be able to successfully decode multiple images in sequence without forcing caller to keep track of and seek to next frame.

The program https://play.golang.org/p/lppyHZftWA is expected to not give any output but decode all stream frames correctly and exit cleanly when hitting io.EOF

What did you see instead?

The program prints an error message as it hits io.ErrUnexpectedEOF

Does this issue reproduce with the latest release (go1.9.1)?

Yes

System details

go version go1.9 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/hnnsgstfssn/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/md/xyw1fqr954lfbtnfc_9ym_0r0000gn/T/go-build236075526=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="0"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version go1.9 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.9
uname -v: Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.12.5
BuildVersion:	16F73
lldb --version: lldb-370.0.42
  Swift-3.1
@as
Copy link
Contributor

@as as commented Oct 6, 2017

https://play.golang.org/p/5j6qu6ofuW

The decoder eats your entire stream 4096 bytes at a time, along with all images in between. You will have to use a bytes.Reader instead of a bytes.Buffer and seek to the starting position of the stream to get this to work how you expect.

That out of the way, I can confirm that image/png works in the scenario above without reading past the next image.

Loading

@hnnsgstfssn
Copy link
Author

@hnnsgstfssn hnnsgstfssn commented Oct 7, 2017

The decoder eats your entire stream 4096 bytes at a time, along with all images in between. You will have to use a bytes.Reader instead of a bytes.Buffer and seek to the starting position of the stream to get this to work how you expect.

Thanks for pointing this out. To be clear, I'm expecting the behaviour of image/png in this situation. That is, the decoder reads no further than the EOI marker for each call to Decode: https://play.golang.org/p/Sf-6q1gxTH.

Loading

@hnnsgstfssn hnnsgstfssn closed this Oct 7, 2017
@hnnsgstfssn hnnsgstfssn reopened this Oct 7, 2017
@as
Copy link
Contributor

@as as commented Oct 7, 2017

Yes, that is what I was trying to point out as well. Reading my last comment again I see how ambiguous the statement was. To paraphrase, the image/jpeg is the only package I have used that over-reads the stream, and I also find this behavior a bit unusual.

Loading

@as
Copy link
Contributor

@as as commented Oct 7, 2017

Upon further inspection of the png, gif, and jpeg packages, I don't think this issue can be easily resolved at the package level.

The data structures are simple for byte formats where a bitstream doesn't cross byte-boundaries (e.g., PNG), and it turns out that streamable PNGs are part of the png standard itself:

https://tools.ietf.org/html/rfc2083#section-2

The jpeg package has a custom reader for decoding the variable length bitstream, it fills the buffer 4k bytes at a time. There is generally no way to stuff those unprocessed bytes back into the underling reader after the decode is done, short of reading the bitstream and detecting the EOI marker to avoid over-reads completely. My inference is that this would set back the intended performance gain because the buffer now needs to examine and understand the data as well.

A container format solves this problem, but MJPEG has no formal standardization that I know of, and a bit of research also shows that other decoders have had this problem too:

https://github.com/search?q=mjpeg&type=Issues&utf8=%E2%9C%93

/cc @nigeltao

Loading

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Oct 12, 2017

Yeah, that diagnosis sounds correct, and not easily solvable. Sorry.

Loading

@mattn
Copy link
Member

@mattn mattn commented Oct 12, 2017

https://github.com/mattn/go-mjpeg

I don't see the problem yet.

Loading

@as
Copy link
Contributor

@as as commented Oct 12, 2017

Just at a cursory glance, it seems like https://github.com/mattn/go-mjpeg expects each jpeg in the stream to have a mime header. Is that correct?

In this issue, there is no header at all, it's a concatenation of multiple jpeg images.

Loading

@bradfitz bradfitz changed the title image/jpeg: Unable to decode MJPEG image/jpeg: Unable to decode concatenated JPEGs (MIME-less "MJPEG") Oct 12, 2017
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Oct 12, 2017

MJPEG typically means the MIME version. I've retitled this bug for clarity.

Loading

@john-dev
Copy link

@john-dev john-dev commented May 17, 2018

Any news on this?
Im in the same situation right now..

Loading

@as
Copy link
Contributor

@as as commented May 17, 2018

@john-dev You need to seek to the beginning of the next image yourself. Here's an example for a specific type of MJPEG

https://github.com/as/video/blob/master/mjpeg/mjpeg.go

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants