abema / go-mp4

Go library for reading and writing MP4 file
https://dev.to/sunfishshogi/go-mp4-golang-library-and-cli-tool-for-mp4-52o1
MIT License
464 stars 30 forks source link

Improve compatibility of esds boxes #123

Closed aler9 closed 2 years ago

aler9 commented 2 years ago

@sunfish-shogi

Firefox and Chrome aren't currently able to read varints with a size different than 4. ISO 14496-1 isn't clear on the matter, although it limits the maximum varint size to 4.

This patch forces varints to have a size of 4, allowing a broader compatibility of esds boxes. Sizes greater than 4 are not supported anymore (but this is dictated by the specification).

References: https://stackoverflow.com/questions/30998150/build-an-esds-box-for-an-mp4-that-firefox-can-play https://github.com/FFmpeg/FFmpeg/blob/0922c6b01bd50f0ce6e659f765c244f6a8f29eb3/libavformat/movenc.c#L646 ISO 14496-1

PS: thank you very much for this library, it allows rtsp-simple-server to produce and read Low-Latency HLS fragments.

sunfish-shogi commented 2 years ago

@aler9 Thank you for your pull request! And thank you for using our library on your project!

I, too, don't know where is the description about 4-bytes limitation in standards. But I confirmed, as you said, some products are implemented in according with that criteria.

I researched Bento4. Bento4 has following codes. https://github.com/axiomatic-systems/Bento4/blob/ffea12e7055ba95b4b907cc140785cfb1c420c64/Source/C%2B%2B/Core/Ap4DescriptorFactory.cpp#L65-L78 https://github.com/axiomatic-systems/Bento4/blob/ffea12e7055ba95b4b907cc140785cfb1c420c64/Source/C%2B%2B/Core/Ap4Expandable.cpp#L87-L103 I think Bento4 accepts 1-4 bytes and writes as same size with input.

I'm still thinking following points.

aler9 commented 2 years ago

If someone use mp4:"varint" tag by user-defined box, this change influences them. Which is better always writing 4 bytes like this PR or providing an option (ex. global function, environment variable)?

I'm not aware of any other ISO box that uses varints - if you want to give users the possibility to use varints without padding inside custom boxes, then a field tag like padding=4 may be useful.

sunfish-shogi commented 2 years ago

I'm not aware of any other ISO box that uses varints

Thank you for your reply. Sure, on my second thoughts this change is unlikely to occur problem. I will run test on GH actions, and merge this PR.