Closed
Bug 852002
Opened 11 years ago
Closed 11 years ago
nestegg loses parser sync after encountering zero length master element
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: kinetik, Assigned: kinetik)
References
()
Details
Attachments
(1 file)
6.74 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
Originally reported in bug 778436 comment 14. Bug URL is a link to the attachment in bug 778436. |+ Segment tracks at 288 | + A track at 300 | + Track number: 1 (track ID for mkvmerge & mkvextract: 0) at 309 | + Track type: audio at 312 | + Track UID: 1009975861 at 315 | + Audio track at 326 | + Codec ID: A_VORBIS at 335 | + CodecPrivate, length 2587 at 346 | + Name: Audio at 2937 Note that there are no subelements of "Audio track at 326". You'd usually see one or more of: SamplingFrequency, OutputSamplingFrequency, Channels. The muxer has written an Audio element (0xE1) with a length of zero, the length is encoded as an 8 byte long uint (0x010000000000), but that's not a problem. After reading that, nestegg expects subelements but does not encounter any, and loses parser sync. 0000140: 404b 3c33 0235 e101 0000 0000 0000 0086 @K<3.5.......... ^^ Audio ^^ Size (0, encoded in 8 bytes) ^^ CodecID 0000150: 8941 5f56 4f52 4249 5300 63a2 4a1b 021e .A_VORBIS.c.J... ^^ Size (9) ^^ ^^^^ ^^^^ ^^^^ ^^^^ CodecID string ^^ CodecPrivate ^^ nestegg parser syncs here after reading Audio element.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•11 years ago
|
||
The cause of this turns out to be simple. For each element read, we peek the element ID and size, then prepare the parser based on the element ID, then read the element. The code that allows peek and read to return the same element without rereading the same data offset is here: https://github.com/kinetiknz/nestegg/blob/master/src/nestegg.c#L835 static int ne_peek_element(nestegg * ctx, uint64_t * id, uint64_t * size) { int r; if (ctx->last_id && ctx->last_size) { if (id) *id = ctx->last_id; if (size) *size = ctx->last_size; return 1; } [...] Read calls peek, the resets the last_id and last_size to tell peek to read a new ID and size next time: static int ne_read_element(nestegg * ctx, uint64_t * id, uint64_t * size) { int r; r = ne_peek_element(ctx, id, size); if (r != 1) return r; ctx->last_id = 0; ctx->last_size = 0; [...] The problem is that the Audio element's size is 0, so (ctx->last_id && ctx->last_size) fails as if the previous call was a read that cleared last_id and last_size. This is the danger of trying to use a data value as a flag, when valid values overlap with values treated as a flag. I'll post a patch shortly.
Assignee | ||
Comment 2•11 years ago
|
||
Upstream fix: https://github.com/kinetiknz/nestegg/commit/ebdbb688fb13dd315fc9d16e6897adb5ee42b7bb
Assignee | ||
Comment 3•11 years ago
|
||
Introduce a dedicated flag for tracking peek/read state rather than overloading the values of last_id and last_size.
Attachment #726437 -
Flags: review?(paul)
Updated•11 years ago
|
Attachment #726437 -
Flags: review?(paul) → review+
Verified that the patch fixes my problem on fedora. Also reviewed the code, looks good to me.
Assignee | ||
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e41b1ed18c2 Thanks for the report, and confirming the fix!
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0e41b1ed18c2
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•