Hello!
On Mon, Nov 21, 2016 at 10:04:25PM +0800, 胡聪 (hucc) wrote:
> Hi,
>
> On Mon, Nov 21, 2016 at 9:07 PM +0300, Maxim Dounin wrote:
>
> >Hello!
> >
> >On Mon, Nov 21, 2016 at 05:40:52PM +0800, 胡聪 (hucc) wrote:
> >
> >> Hi,
> >>
> >> # HG changeset patch
> >> # User hucongcong <hucon...@foxmail.com>
> >> # Date 1479719580 -28800
> >> # Mon Nov 21 17:13:00 2016 +0800
> >> # Node ID f43feabe2e45ec102ffbe36428ccb504ff1526d2
> >> # Parent 6a26016e9a138102798a7ec3e74747fbd6018f82
> >> Mp4: fixed setting wrong mdat atom size in very rare cases.
> >>
> >> diff -r 6a26016e9a13 -r f43feabe2e45 src/http/modules/ngx_http_mp4_module.c
> >> --- a/src/http/modules/ngx_http_mp4_module.c Tue Nov 15 18:11:46 2016
> >> +0300
> >> +++ b/src/http/modules/ngx_http_mp4_module.c Mon Nov 21 17:13:00 2016
> >> +0800
> >> @@ -1229,7 +1229,7 @@ ngx_http_mp4_update_mdat_atom(ngx_http_m
> >>
> >> atom_header = mp4->mdat_atom_header;
> >>
> >> - if ((uint64_t) atom_data_size > (uint64_t) 0xffffffff) {
> >> + if ((uint64_t) atom_data_size > (uint64_t) 0xfffffff7) {
> >> atom_size = 1;
> >> atom_header_size = sizeof(ngx_mp4_atom_header64_t);
> >> ngx_mp4_set_64value(atom_header + sizeof(ngx_mp4_atom_header_t),
> >
> >You may want to elaborate more about what are you trying to fix
> >here.
> >
> >Current code uses 32-bit atom headers for sizes up to maximum
> >unsigned 32-bit value, 0xffffffff. And this seems to perfectly
> >agree with the specification.
>
> Well, atom size is the sum of atom header size and atom data size.
> And the specification says that the first 4 bytes are set to one
> when the atom size is greater than 0xffffffff, which means atom
> header size should be considered when the comparison takes place
> between atom data size and the maximum unsigned 32-bit value.
> We can image that atom_data_size is 0xfffffff9, and the first 4 bytes
> will be set to the sum of sizeof(ngx_mp4_atom_header_t) and
> atom_data_size, then the error is obvious.
Ah, ok, I see the problem now. Please clarify things in the
commit log as well.
It also make sense to use sizeof() explicitly, instead of
introducing a magic number, e.g.:
@@ -1229,7 +1229,9 @@ ngx_http_mp4_update_mdat_atom(ngx_http_m
atom_header = mp4->mdat_atom_header;
- if ((uint64_t) atom_data_size > (uint64_t) 0xffffffff) {
+ if ((uint64_t) atom_data_size > (uint64_t) 0xffffffff
+ - sizeof(ngx_mp4_atom_header_t))
+ {
atom_size = 1;
atom_header_size = sizeof(ngx_mp4_atom_header64_t);
ngx_mp4_set_64value(atom_header + sizeof(ngx_mp4_atom_header_t),
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
