#266 closed defect (fixed)
mp4 sendfile errors
Reported by: | Gernot Vormayr | Owned by: | Maxim Dounin |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-module | Version: | 1.2.x |
Keywords: | mp4 | Cc: | |
uname -a: | Linux k817 2.6.32-5-amd64 #1 SMP Mon Oct 3 03:59:20 UTC 2011 x86_64 GNU/Linux | ||
nginx -V: |
nginx version: nginx/1.2.6
TLS SNI support enabled configure arguments: --prefix=/etc/nginx/ --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-http_ssl_module --with-http_realip_module --with-http_addition_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gzip_static_module --with-http_random_index_module --with-http_secure_link_module --with-http_stub_status_module --with-mail --with-mail_ssl_module --with-file-aio --with-ipv6 (package from http://nginx.org/packages/debian/ squeeze nginx) |
Description
After trying out the mp4 module I received some videos didn't work and I got strange errors like:
2012/12/28 20:16:02 [alert] 17920#0: *39954 sendfile() failed (22: Invalid argument) while sending mp4 to client, client: a.b.c.d, server: , request: "GET /myfile.mp4?start=26.89 HTTP/1.0", host: "a.b.c.d"
Well yeah I tried turning off sendfile:
2012/12/28 20:07:08 [emerg] 17864#0: *28769 malloc(18446744073606090997) failed (12: Cannot allocate memory) while sending mp4 to client, client: a.b.c.d, server: , request: "GET /myfile.mp4?start=26.89 HTTP/1.0", host: "a.b.c.d"
Well since 16PB of ram are a bit too expensive for us I dug into the mp4 code and the mp4 file. Turning on debugging led (as expected) to some nice negative values in the mp4 module. After some more digging I found out, that ngx_http_mp4_read_mdat_atom
gets called with a zero atom_data_size
which is correct according to the video:
xxd -s 714204 -l 8 myfile.mp4 00ae5dc: 0000 0008 6d64 6174 ....mdat
So after some googling I read somewhere that this means that the mdat atom spans the rest of the file? Well I don't know anything about mp4, but I suspect one of you guys knows :)
I believed the text and after applying the patch the file works.
Attachments (2)
Change History (17)
by , 12 years ago
Attachment: | mp4_fix_empty_mdat.patch added |
---|
comment:1 by , 12 years ago
Priority: | critical → minor |
---|---|
Status: | new → accepted |
Yep, size 0 is a special value, ISO Base Media File Format specification (ISO/IEC 14496-12:2008) says:
size is an integer that specifies the number of bytes in this box, including all its fields and contained boxes; if size is 1 then the actual size is in the field largesize; if size is 0, then this box is the last one in the file, and its contents extend to the end of the file (normally only used for a Media Data Box)
But the file in question seems to claim size 8 instead of 0, which is wrong. Note that function you've modified is called with a size of a atom minus it's header.
The error you see is a result of a mismatch between the file metadata in moov atom and empty mdat atom (only 8 bytes of the header). It is not really a problem per se (as mp4 file is broken), though it certainly needs some better diagnostics.
comment:2 by , 12 years ago
Thanks for the info. I'll have another look at the file and try to understand the standard. I'll come back as soon as I find out more.
Actually the file has another mdat atom right after the empty one ->
xxd -l 16 -s 714204 myfile.mp4 00ae5dc: 0000 0008 6d64 6174 0679 3ea2 6d64 6174 ....mdat.y>.mdat
Hmm I don't have time right now - but I think I can have another look at it at the weekend.
comment:3 by , 12 years ago
Ok now I think I know what's going wrong (well in nginx - not the mp4 - I dunno why there is an empty mdat):
ISO Base Media File Format specification (ISO/IEC 14496-12:2008) says:
A presentation may contain zero or more Media Data Boxes.
There may be any number of these boxes in the file (including zero, if all the media data is in other files). The metadata refers to media data by its absolute offset within the file (see subclause 8.7.5, the Chunk Offset Box); so Media Data Box headers and free space may easily be skipped, and files without any box structure may also be referenced and used.
As far as I've read the code I think nginx mp4 module only supports files with one mdat atom. Some of my files seem to have multiple mdat atoms which is ok according to the standard.
So I played a bit around and replaced
if (mp4->trak.nelts) { /* skip atoms after mdat atom */ mp4->offset = mp4->end; } else { ngx_mp4_atom_next(mp4, atom_data_size); }
with
ngx_mp4_atom_next(mp4, atom_data_size);
in ngx_http_mp4_module.c.
The video works now, but I wonder if this is the right course of action here. After this change sane files with only one mdat atom should behave the same. In files with multiple mdat atoms mp4->mdat_data.buf->file_last
should point to the end of the last mdat atom. I'm not sure, but if I didn't miss anything, this shouldn't do anything bad, because as far as I understand the metadata refers to the media data via offsets within the file anyways.
What was the reasoning behind /* skip atoms after mdat atom */
?
comment:4 by , 12 years ago
Owner: | set to |
---|---|
Status: | accepted → assigned |
comment:5 by , 12 years ago
While ISO BMFF indeed allows more than one mdat atom, the F4V specification by Adobe (i.e. what Adobe Flash understand) states that there should be only one mdat atom. So you probably don't want multiple mdat atoms in your files anyway.
I don't think we need to support multiple mdat atoms given the above as it more likely cause harm than good, but we may consider skipping such empty mdat atoms if they are common enough. Could you please elaborate a bit more on problematic files origin and how common they are?
(Note that with your change you essentially skip all but last mdat atoms.)
comment:6 by , 12 years ago
The files in question are directly from our client. I'm going to ask them how they got created.
We also generate smaller versions of those files with ffmpeg and I looked at them and noticed that they have a size 8 free
atom just before the mdat
atom. As far as I found out from the ffmpeg source they put either a free
or a wide
atom just before the mdat
so that if the mdat
s size is bigger than those 32 bits the free
/wide
atom can be overwritten and nothing has to be recalculated. Maybe the video creation software wanted to do the same but chose to use mdat
for this placeholder.
ffmpeg reports this as the metadata:
Metadata: major_brand : mp42 minor_version : 1 compatible_brands: mp42avc1 creation_time : 2012-02-24 14:31:57 Duration: 00:19:44.25, start: 0.000000, bitrate: 738 kb/s Stream #0:0(eng): Audio: aac (mp4a / 0x6134706D), 48000 Hz, stereo, s16, 125 kb/s Metadata: creation_time : 2012-02-24 14:31:58 handler_name : Apple Ton Mediensteuerung Stream #0:1(eng): Video: h264 (Main) (avc1 / 0x31637661), yuv420p, 640x360, 608 kb/s, SAR 1:1 DAR 16:9, 25 fps, 25 tbr, 25 tbn, 50 tbc Metadata: creation_time : 2012-02-24 14:31:58 handler_name : Apple Video Mediensteuerung
(maybe some older version of quicktime?)
comment:7 by , 12 years ago
Hmm only answer I could get: "Coder" and "Final Cut".
I'd say it would be better to fix the videos than implement a fix for nginx. Maybe add a note somewhere that nginx only works with one mdat atom?
comment:8 by , 12 years ago
At least we want better error in such cases, which probably will be enough. As documentation already states that the mp4 module is intended to work with Flash, I don't think it worth specifically outlining the should be only one mdat atom as it's already required by the f4v specification.
comment:9 by , 12 years ago
Ahh better error reporting is a good Idea - I'll have a look into it at the weekend.
follow-up: 12 comment:10 by , 12 years ago
Hello, I have a similar error in my log: *3 sendfile() failed (32: Broken pipe) while sending mp4 to client, client: , server: , request: "GET /video/basic-a9.mp4 HTTP/1.1", host: "a.b.c.d"
uname -a = Linux hostname 2.6.32-45-generic #101-Ubuntu SMP Mon Dec 3 15:39:38 UTC 2012 x86_64 GNU/Linux
and nginx config details are:
nginx version: nginx/1.2.6built by gcc 4.4.3 (Ubuntu 4.4.3-4ubuntu5.1) configure arguments: --prefix=/etc/nginx/ --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-http_realip_module --with-http_addition_module --with-http_sub_module --with-http_dav_module --with-http_mp4_module --with-http_gzip_static_module --with-http_random_index_module --with-http_secure_link_module --with-http_stub_status_module --with-file-aio --with-debug
When i disable sendfile, the error goes away , but the mp4 module does not process the video file - I do not see the log messages from inside the mp4_process function. This used to work last year (May was the last i tried).
by , 12 years ago
Attachment: | mp4_mdat_atom_check.patch added |
---|
comment:11 by , 12 years ago
The attached patch throws an error if the offset is outside the mdat atom - I couldn't think of a good way of implementing it in ngx_http_mp4_update_mdat_atom
so i implemented it in ngx_http_mp4_process
.
mp4_mdat_atom_check.patch
It works with my strange files.
comment:12 by , 12 years ago
Replying to B Ganesh:
Hello, I have a similar error in my log: *3 sendfile() failed (32: Broken pipe) while sending mp4 to client, client: , server: , request: "GET /video/basic-a9.mp4 HTTP/1.1", host: "a.b.c.d"
uname -a = Linux hostname 2.6.32-45-generic #101-Ubuntu SMP Mon Dec 3 15:39:38 UTC 2012 x86_64 GNU/Linux
and nginx config details are:
nginx version: nginx/1.2.6built by gcc 4.4.3 (Ubuntu 4.4.3-4ubuntu5.1) configure arguments: --prefix=/etc/nginx/ --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-http_realip_module --with-http_addition_module --with-http_sub_module --with-http_dav_module --with-http_mp4_module --with-http_gzip_static_module --with-http_random_index_module --with-http_secure_link_module --with-http_stub_status_module --with-file-aio --with-debug
When i disable sendfile, the error goes away , but the mp4 module does not process the video file - I do not see the log messages from inside the mp4_process function. This used to work last year (May was the last i tried).
Well there was no ?start=
parameter so there is no need to process it with the mp4 module. As far as I know it gets served like any other file. Broken pipe
sounds like a different problem.
comment:14 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I've committed the patch with minor changes, thanks.
set file_last of mdat atom to file end if atom size is 0