search

mikebrady / shairport-sync-metadata-reader

Sample Shairport Sync Metadata Player
MIT License
126 stars 33 forks source link

bad decoding #3

Closed rkam closed 9 years ago

rkam commented 9 years ago

Hi,

I am playing around with the meta-data code (looking at producing regular song position updates) and I noticed sometimes this program is incorrectly decoding.

I looked into it and when you made the changes to remove malloc, you used a static buffer for the decoding_table. However, you used 64 for the size. If you look at line 57, the index into the decoding_table is the value of the element in the encoding_table at the loop index. Since that value is ascii, it can range from 0..255. So, the size should be 256.

You could optimize though - the largest ascii value in the encoding array is 'z' -, so a size of 123 should work. (I just went with 256, though.)

Thanks for all this, it's great stuff.

mikebrady commented 9 years ago

Thanks for that – it took me a few minutes to realise that you're right. I'll make a quick fix now and I might initialise the array to zero (tidy mind syndrome) in a later fix.

rkam commented 9 years ago

No problem. Yeah, it took me a while too. At first, I looked at it and thought, yeah, 64 is right.. It's that loop limit that got me.

It doesn't need zero'ing though. It's a global (so in the BSS) and is auto-zero'ed when exec'ed.

mikebrady commented 9 years ago

Cool, I didn't know that about auto zeroing.

rkam commented 9 years ago

Yep. Every (non-initialized) global variable is zeroed.

It's just stuff malloc'ed or allocated on the stack (i.e. defined in functions) that aren't.

Also, note that (non-initialized) static variables defined inside a function are zeroed too.

Trivia: Uninitialized globals are put into the BSS section of the program and initialized globals are put into the DATA section of the file/program (and, in some implementations, const data is put into a separate read-only data section).

BSS is not stored in the file, just the length of it. When the program is started, space is then allocated (and zeroed, if not already zero).

On Sun, Aug 9, 2015 at 12:25 PM, Mike Brady notifications@github.com wrote:

Cool, I didn't know that about auto zeroing.

— Reply to this email directly or view it on GitHub https://github.com/mikebrady/shairport-sync-metadata-reader/issues/3#issuecomment-129227593 .

mikebrady commented 9 years ago

I'm going to close this – feel free to reopen if there are developments.