RReverser / mpegts

Javascript HTTP Live Streaming realtime converter and player
http://rreverser.github.io/mpegts/
MIT License
838 stars 155 forks source link

Uncaught TypeError: Unexpected value (71 !== 0) #6

Closed maxlapshin closed 10 years ago

maxlapshin commented 10 years ago

Uncaught TypeError: Unexpected value (71 !== 0)

Tried to read where this error happens, looks that in jBinary when handling strict const.

The only strict const 0 found in mpegts is a PES start code, but jBinary doesn't report name of field =( So I failed to understand why do MPEG-TS start code is read when 0 was awaited.

Problem happens on attached file, which is played on any HLS player: http://images.maxidoors.ru/47-10000.ts

Can I help with something else? It is not very easy to debug jBinary because I'm unfamiliar with it, but I can help with any video part of this code.

RReverser commented 10 years ago

Thanks for feedback. I planned to improve debugging in jBinary for pretty long time, but that wasn't highly requested, so I didn't spend any time on that. For some architectural reasons, it's not easy (and even not always possible) to detect field name being currently processed, in runtime. Instead, I just implemented and published jBinary v2.0.7 that utilizes Chrome / Firefox Dev Tools that allows to give debug name to closures. Now stack of that error looks like following:

screenshot

I believe this should help in debugging since you can see type signatures, field names and byte offsets inside current jBinary instance and so faster detect source of problem.

Now regarding video sample - as stated on main page of repo, I implemented functionality heavily relying on official specifications (which are sometimes pretty crazy things to read and translate to code) and playing with video samples in HEX editor.

Apparently, a lot of tools have own deviations from specs, and specs theirselves are not always clear about edge cases, so there are deviations between some real samples and expected structures which result to errors like above. I tried to cover functionality for properly constructed chunks&streams, but, obviously, couldn't cover all the possible deviations since almost each sample that results in error needs separate studying for understanding source of problem and fixing / workarounding in structures.

If you could help to detect source of problem and fix structures correspondingly, that would be highly appreciated. Let me know if you need anything more from me (I hope after these debugging improvements it should go somewhat easier).

RReverser commented 10 years ago

@maxlapshin Did you try to figure out reason of bug?

maxlapshin commented 10 years ago

I failed to do it.

It is very hard for me to get through javascript stacktraces =(

On May 12, 2014, at 5:59 PM, Ingvar Stepanyan notifications@github.com wrote:

@maxlapshin Did you try to figure out reason of bug?

— Reply to this email directly or view it on GitHub.

RReverser commented 10 years ago

Didn't descriptive stack traces improvement help?

RReverser commented 10 years ago

@maxlapshin I've added nodejs branch with adapted code and your sample so you can run

$ node .

inside and it will try to convert sample.ts (your file) to sample.mp4. Currently it throws the same error stack:

2014-05-12_19-46-13

Offset that it shows, is inside temporary pesStream instance of jBinary which contains (at least, should contain) only collected PES packets. You can insert

require('fs').writeFileSync('sample.pes', pesStream.read('blob', 0));

at line 32 of index.js to save PES stream only as stream.pes synchronously (so it will be completely saved before error is thrown) and open this file in HEX editor. After navigating to given offset (364570) you can see that there is really Transport Stream Packet header:

2014-05-12_19-56-54

Why wasn't it filtered out and appeared in PES stream, needs additional investigations and your help would be highly appreciated.

maxlapshin commented 10 years ago

I don't see any easy-to-find error in https://github.com/RReverser/mpegts/blob/gh-pages/mpegts-to-mp4/mpegts.js#L157

Here is my code for extracting ts payload from mpegts:

ts_payload(<<16#47, _:18, 0:1, 1:1, _:4, Payload:184/binary, _/binary>>)  -> 
  Payload;

ts_payload(<<16#47, _:18, 1:1, 1:1, _:4, AdaptationLength, _AdaptationField:AdaptationLength/binary, Rest/binary>>) when AdaptationLength =< 183 ->
  Length = 183 - AdaptationLength,
  case Rest of
    <<Payload:Length/binary, _/binary>> -> Payload;
    _ -> eof
  end;

ts_payload(<<16#47, _:18, 1:1, 1:1, _:4, AdaptationLength, _/binary>>) ->
  {error, {invalid_adaptation_field, AdaptationLength}};

ts_payload(<<16#47, _:18, 1:1, 0:1, _:4, _/binary>>) ->
  {error, only_adaptation_field};

ts_payload(<<16#47, _:18, 0:1, 0:1, _:4, _:184/binary, _/binary>>)  -> 
  erlang:error(invalid_payload).

_:18 syntax means "skip 18 bits", 0:1 means "one bit must be 0".

I think that it will help if you output DTS of each extracted PES packet.

Perhaps, problem is here:

https://github.com/RReverser/mpegts/blob/gh-pages/mpegts-to-mp4/pes.js#L57

H264 MUST have forbidden bit set to zero, but not always have it.

Better to use bit 2 in mpegts, which is start.

I can also share my test suite for mpegts decoder library, there are about 30 MB of different files from various muxers.

RReverser commented 10 years ago

I think that it will help if you output DTS of each extracted PES packet.

That's easy to add - just follow structure defined in pes.js and put

if (packet._hasDTS) console.log(packet.dts);

on line 35 of index.js, right after

var packet = pesStream.read('PESPacket');

Last DTS it outputs is -2085236713 but not sure how it helps here.

Better to use bit 2 in mpegts, which is start.

I'm not sure what do you mean. Is this bit part of NAL unit type? When length is not explicitly given in PESPacket, I'm looking for 00 00 01 [code] pattern, where code has forbidden bit set so it successfully finds packets of audio (C0), video (E0) and other streams.

maxlapshin commented 10 years ago

I mean that you'd better use flag payloadStart: https://github.com/RReverser/mpegts/blob/gh-pages/mpegts-to-mp4/mpegts.js#L162

to look for next PES beginning.

Your detection method may fail because sometimes H264 NAL units has forbidden bit set to 1 and it is generally ok, because you cannot do anything with it.

RReverser commented 10 years ago

I mean that you'd better use flag payloadStart: https://github.com/RReverser/mpegts/blob/gh-pages/mpegts-to-mp4/mpegts.js#L162 to look for next PES beginning

But payloadStart is part of Transport Stream header so detection based on it may fail on cases when TS packet's raw stream contains two PES packets (and that's possible according to spec).

Also, fixing end detection could help in this particular case but original problem is definitely not in PESPacket structures but somewhere in Packet types defined in https://github.com/RReverser/mpegts/blob/gh-pages/mpegts-to-mp4/mpegts.js since, as we see when saving sample.pes as I described above, Transport Stream packet somehow appears in PES stream before it even begins being parsed. That could happen only somewhere in initial gathering of PES stream: https://github.com/RReverser/mpegts/blob/nodejs/index.js#L16-L25

Probably payload._rawStream is not extracted correctly in some cases so it gathers next TS packet's header as part of raw stream of current one.

maxlapshin commented 10 years ago

Check size of _rawStream here.

It must be usually 184 and sometimes a bit smaller for adaptation field.

RReverser commented 10 years ago

Yeah, as you can see from https://github.com/RReverser/mpegts/blob/nodejs/mpegts-to-mp4/mpegts.js#L64-L66, it's actually expected to be 188 minus header size.

And yes, there are 2 TS packets with raw streams that for some reason have length of 188 bytes.

RReverser commented 10 years ago

Interesting, just understood that ES length calculation was not totally correct since streams may start not on 188x offsets, fixed and got another error on file.write call in index.js: 2014-05-12_21-12-13

Not finished, but good progress :)

maxlapshin commented 10 years ago

I completely ignore ES length in flussonic. It is usually useless.

RReverser commented 10 years ago

I mean not field value but real calculated length of raw stream that I'm using in each TS packet.

RReverser commented 10 years ago

So that error is fixed now in c67216c75585c505e4dd710115c5605a7a59ea82.

RReverser commented 10 years ago

And improved stack traces by filtering out not interesting parts so now we have error with stack that looks like: 2014-05-12_21-51-14

RReverser commented 10 years ago

Nice, looks like problem is that Node.js Buffer can't write given creation_time as UInt32 since it's out of bounds allowed for this type.

RReverser commented 10 years ago

@maxlapshin Ok, looks like this final error was only Node.js related and conversion itself was fixed earlier in c67216c so your video sample is now converted successfully. Thanks for investigations and help!

RReverser commented 10 years ago

I can also share my test suite for mpegts decoder library, there are about 30 MB of different files from various muxers.

@maxlapshin Could you actually share this test suite so I could look for other problems here? Thanks.

RReverser commented 10 years ago

@maxlapshin Ping.

Jaetoh commented 8 years ago

Hi ! I try to upload a file .ts with the npm package but jBinary send me back this error

[TypeError: Unexpected value (8 !== 0).]

I really don't succed to fix it even with this topic. Could anyone help me ? Thanks