mp4 sendfile errors

[Gernot Vormayr]

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.

[Gernot Vormayr]

set file_last of mdat atom to file end if atom size is 0

[Maxim Dounin]

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.

[Gernot Vormayr]

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.

[Gernot Vormayr]

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 */?

[Maxim Dounin]

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.)

[Gernot Vormayr]

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 mdats 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

[Gernot Vormayr]

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?

[Maxim Dounin]

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.

[Gernot Vormayr]

Ahh better error reporting is a good Idea - I'll have a look into it at the weekend.

[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).

[Gernot Vormayr]

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.

[Gernot Vormayr]

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.

[sync]

In 5098/nginx:

(The changeset message doesn't reference this ticket)

[Maxim Dounin]

I've committed the patch with minor changes, thanks.

[Maxim Dounin]

In 5156/nginx:

(The changeset message doesn't reference this ticket)