Skip to content

implicit SBR data#7

Open
dieterstueken wants to merge 22 commits intoDV8FromTheWorld:masterfrom
dieterstueken:master
Open

implicit SBR data#7
dieterstueken wants to merge 22 commits intoDV8FromTheWorld:masterfrom
dieterstueken:master

Conversation

@dieterstueken
Copy link
Copy Markdown

@dieterstueken dieterstueken commented Dec 29, 2017

I tried to play some internet radio stream containing implicit SBR and found that decodeSBR() did not work.
The given count was changed from bytes to bits some day, but this was not correctly propagated.
SBR.decode() reads the extension_type a second time instead of using boolean crc.
Then I found the frequency for implicit SBR should be doubled under some circumstances (don't know if done correctly for all cases).

I forked a java8 branch and made some changes to the logging framework (which currently is a mess and too slow without java8). But I don't know if you want to use java8. Thus I extracted just the bugs back into my main branch.

todo:

I found that the first 16 frames did not contain any sbr_header. Currently no sbr_data is read without sbr_header, but the size of the SBR payload indicates there may be some sbr_data.
... I'm working on this.

Currently SBR.decode() just absorbs any additional data. I think this is not correct.
After reading the first payload there may follow additional payloads.
The original code returned the #of bytes processed. This seems to be broken as int result is mixed up with some return status code (from the C code).

@stigc
Copy link
Copy Markdown

stigc commented Mar 12, 2018

I have tested this on some SBR files and it works fine. If I disable SBR in DecoderConfig, the sound plays in double tempo, maybe something is wrong here?

	sbrEnabled = false;

@dieterstueken
Copy link
Copy Markdown
Author

do you have some example file or link playing in double tempo?
I only have a limited set of samples. I'm just started to understand AAC,
but seems there are zillions of different ways to encode AAC.

@stigc
Copy link
Copy Markdown

stigc commented Mar 27, 2018

Actually I cannot provoke the error now. I have to try later. But I have trouble playing this stream, does it work for you? http://51.254.29.40:80/stream3

I hear a lot of glitches.

@dieterstueken
Copy link
Copy Markdown
Author

yes, sounds bad.

The code is a transcription of faad2 (//salsa.debian.org/multimedia-team/faad2)
which is capable to decode your stream. But it takes hours of debugging to locate the problem.

I also found other problems when decoding an mp4 container...

@stigc
Copy link
Copy Markdown

stigc commented Mar 28, 2018

Foobar2000 also plays it flawless

@dieterstueken
Copy link
Copy Markdown
Author

the left channel returns with all values NaN at:
SyntacticElements.java#L308

will look at it further tomorrow.

@dieterstueken
Copy link
Copy Markdown
Author

OK, found a very effective random generator to produce spectral noise.

randomState *= 1664525+1013904223;

since 1664525+1013904223=1015568748 is even, after a few steps it happens that:

-1073741824 * (1664525+1013904223) == 0

At this point all future values become NaN.

@dieterstueken
Copy link
Copy Markdown
Author

two more fixes:
MetaBox seems to carry 12 byte of payload.
(did not found any documentation about yet, but works for me)

Random generator fixed according to:
Numerical Recipes

@stigc
Copy link
Copy Markdown

stigc commented Mar 29, 2018

Great work this plays flawless now http://51.254.29.40:80/stream3. However the "
MetaBox with 12 byte payload" fix throws exception on all my local files (LC and SBR).

java.io.IOException: error while decoding box 'appl' at offset 30489: box too large for parent
at net.sourceforge.jaad.mp4.boxes.BoxFactory.parseBox(BoxFactory.java:347)
at net.sourceforge.jaad.mp4.boxes.BoxImpl.readChildren(BoxImpl.java:110)
at net.sourceforge.jaad.mp4.boxes.impl.MetaBox.decode(MetaBox.java:27)
at net.sourceforge.jaad.mp4.boxes.BoxFactory.parseBox(BoxFactory.java:353)
at net.sourceforge.jaad.mp4.boxes.BoxImpl.readChildren(BoxImpl.java:110)
at net.sourceforge.jaad.mp4.boxes.BoxFactory.parseBox(BoxFactory.java:357)
at net.sourceforge.jaad.mp4.boxes.BoxImpl.readChildren(BoxImpl.java:110)
at net.sourceforge.jaad.mp4.boxes.BoxFactory.parseBox(BoxFactory.java:357)
at net.sourceforge.jaad.mp4.MP4Container.readContent(MP4Container.java:90)
at net.sourceforge.jaad.mp4.MP4Container.(MP4Container.java:74)
at dk.stigc.javatunes.audioplayer.player.AacMp4Player.decode(AacMp4Player.java:21)

@dieterstueken
Copy link
Copy Markdown
Author

Hi Stig,

do you have any example file or link?
I could not find any documentation about the meta box payload.
I have a single file containing a meta box of 12 byte, but I don't know anything
about its meaning. May be the payload depends on it's context.

@stigc
Copy link
Copy Markdown

stigc commented Mar 30, 2018

Try this one http://stigc.dk/SBR 02 Loca (feat. Dizzee Rascal).m4a

@dieterstueken
Copy link
Copy Markdown
Author

those are exactly the bugs I fixed.
However my pull request is still pending and was not accepted since.
Try to use my forked repository instead:
https://github.com/dieterstueken/JAADec/network

@stigc
Copy link
Copy Markdown

stigc commented Mar 30, 2018

I am using you forked version and this commit breaks the linked .m4a file
dieterstueken@e727658

If i comment out the skipBytes like this, it works again.

@OverRide
public void decode(MP4InputStream in) throws IOException {
// some encoders (such as Android's MexiaMuxer) do not include
// the version and flags fields in the meta box, instead going
// directly to the hdlr box
long possibleType = in.peekBytes(8) & 0xFFFFFFFFL;
if(possibleType != BoxTypes.HANDLER_BOX){
super.decode(in);
//in.skipBytes(8);
}
readChildren(in);
}

@dieterstueken
Copy link
Copy Markdown
Author

OK, give it a try tomorrow.
For me it works just the opposite way, don't know.
May be something went wrong with my commit...

@DV8FromTheWorld
Copy link
Copy Markdown
Owner

I haven't accepted the PR yet as I'm waiting to see what happens with the issues brought up thus far.

@dieterstueken
Copy link
Copy Markdown
Author

strange:
using your example http://stigc.dk/SBR 02 Loca (feat. Dizzee Rascal).m4a

without in.skipBytes(8) I get:

INFO: BoxFactory: unknown box type: ��>; position: 19190
Exception in thread "main" java.io.IOException: error while decoding box ' ' at offset 19391: box too large for parent

adding the patch works for me.

@dieterstueken
Copy link
Copy Markdown
Author

Hi Austin,

meanwhile this PR contains a couple of issues already.
Should I split it up into separate small PR's (creating a local branch for each?)

Dieter.

@stigc
Copy link
Copy Markdown

stigc commented Mar 31, 2018

I found the problem. I am using a non seekable inputstream. The seek operation is broken in MP4InputStream. When using a FileInputStream then getOffset() will change but not when using a RandomAccessFile. That is why it works for you skipping 8 bytes. I don't have a fix for it, many things seems a little weird. Why is that public int peek() will remove bytes from the peek collection?

Seeking was added here e0bc448#diff-82931d61e44f3dbc5c7a4e57237f4843

@dieterstueken
Copy link
Copy Markdown
Author

Confirmed:

there is no 8 byte payload on MetaBox. Instead MP4InputStream.peek() is broken. Unfortunately MP4InputStream became a split brain class which tries to handle sequential and seekable input streams simultaneously. This leads to a mess of if/else cascades. While a sequential stream can not be rewound after a read ahead, it is done with the random access stream. Thus both cases disagree about what the actual file position is after a read ahead.
Instead of introducing some more if/else cascades to solve each individual case, I prefer to handle the seekable stream just like the sequential stream: without rewinding after a read ahead.

A better solution might be to turn MP4InputStream into an Interface with separate implementations. But I don't know, if overcoming the procedural architecture of JAAD is undesired :-)

@stigc
Copy link
Copy Markdown

stigc commented Apr 3, 2018

Commit looks fine to me and all my test audio files plays fine.

jimm98y added a commit to jimm98y/SharpRTSPtoWebRTC that referenced this pull request Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants