Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged support for audio/L16 mimetype from andrewfg PR 251 #777

Merged
merged 2 commits into from
Mar 20, 2012

Conversation

jmarshallnz
Copy link
Contributor

c0diq thinks this should be Eden material.

As far as I can tell, the changes are both sane (fixes a bug) and have minimal impact.

Tested build on osx. Needs build test on win32/ios/atv2, plus general signoff.

@wsoltys
Copy link

wsoltys commented Mar 17, 2012

compiles fine on win32 but two things I noticed:

  1. l16 and pcm extensions aren't added
  2. I didn't found good/right test files for it but with the wavs I've tested I just get noise (PCM S16 LE araw, PCM MU-LAW mlaw) even though the content type equals audio/l16.

At least the PCM S16 LE araw plays fine without this patch.

@Memphiz
Copy link
Member

Memphiz commented Mar 17, 2012

Builds for ios/atv2 and linux. But i'm too struggling with testfiles and test procedure here.

@c0diq
Copy link
Contributor

c0diq commented Mar 17, 2012

http://tools.ietf.org/html/rfc2586

audio/L16 L16 denotes uncompressed audio data, using 16-bit signed representation in twos-complement notation and network byte order.

@c0diq
Copy link
Contributor

c0diq commented Mar 17, 2012

is it possible that we should be using Endian_SwapLE16 instead of Endian_Swap?

@c0diq
Copy link
Contributor

c0diq commented Mar 17, 2012

I think we should be using ntohs instead of Endian_Swap to make sure it works on all system. Input is expected to be BigEndian (network byte order).

@c0diq
Copy link
Contributor

c0diq commented Mar 17, 2012

To test find a raw pcm in BigEndian format.

@elupus
Copy link
Contributor

elupus commented Mar 18, 2012

Fine by me.

@c0diq
Copy link
Contributor

c0diq commented Mar 18, 2012

I think it should use ntohs instead of EndianzSwap otherwise it might not work on Linux system right?

-s

On Mar 18, 2012, at 4:43 AM, Joakim Plate reply@reply.github.com wrote:

Fine by me.


Reply to this email directly or view it on GitHub:
#777 (comment)

@jmarshallnz
Copy link
Contributor Author

EndianSwap16() definitely isn't right - it'll swap it to LE regardless of host. We want the equivalent of EndianSwapBE16 instead (currently non-existent), which won't swap on a BE host.

I personally don't mind if ntohs is used if it's universally available, but given it's not currently used in the XBMC codebase, perhaps using EndianSwapBE16() is better - will append a patch.

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

ntohs is used in many places in xbmc.
I don't have a Linux env to try. Both osx and win are little Endian, so I don't know why you think EndianSwapBE16 be better?

-s

On Mar 18, 2012, at 5:27 PM, jmarshallnz reply@reply.github.com wrote:

EndianSwap16() definitely isn't right - it'll swap it to LE regardless of host. We want the equivalent of EndianSwapBE16 instead (currently non-existent), which won't swap on a BE host.

I personally don't mind if ntohs is used if it's universally available, but given it's not currently used in the XBMC codebase, perhaps using EndianSwapBE16() is better - will append a patch.


Reply to this email directly or view it on GitHub:
#777 (comment)

@jmarshallnz
Copy link
Contributor Author

I probably typo'd the random string of characters in my grep :p

I don't have any preference whatsoever - I merely took the route of least resistance that didn't involve having to play with headers (I think <netinet/in.h> is OK on win32 but didn't want to be bothered looking into it). You're welcome to change it to use ntohs if you prefer - send details and I'll slot it in.

It'd be great if you (or someone) could supply a sample file so that others can test it out - they appear to be thin on the ground.

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

You are right. I created a EndianSwapBE16 function. I also fixed an issue where wav file were playing noise now. That's because wav somehow end up with audio/l16 mimetype which triggered the PCMCodec incorrectly. Now why wav have this mimetype I don't get it. I have the fixes, should I close this request and pull a new one?

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

ok I see you did the same thing ;-)

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

Added a sample .l16 file for testing http://cl.ly/F8Da

@jmarshallnz
Copy link
Contributor Author

Just link me the fixes and I'll pull 'em in to this branch.

@jmarshallnz
Copy link
Contributor Author

wave files seem to work fine for me. Have checked the .l16 file you linked to - works fine.

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

Excellent. Then we're good.

-s

Sent from my iPad

On Mar 18, 2012, at 10:11 PM, jmarshallnz reply@reply.github.com wrote:

wave files seem to work fine for me. Have checked the .l16 file you linked to - works fine.


Reply to this email directly or view it on GitHub:
#777 (comment)

@bossanova808
Copy link
Contributor

Does the PR wholly subsume PR251? I believe this will enable direct play of lossless streams from logitech music server (squeezebox ecosystem) if so, so I am curious as my add on is in this area...

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

yes it does. What's lossless streams from logitech?

@Memphiz
Copy link
Member

Memphiz commented Mar 19, 2012

Confirmed, that that test file didn't play before this PR (xbmc just crashed) and played after applying the patch. Tested on OSX and iOS.

So basically funk is flowing through my veins now :)

@bossanova808
Copy link
Contributor

Basically Logitech Media Server is a music (I think it does pictures and
video now too) app from Logitech to support the Squeezebox line of network
audio players. And apparently LMS supplies streams in this format when
streaming lossless files like flac etc.

A long time ago Squeezeboxes were the most awesome network music players,
like a Sonos but a lot cheaper, more flexible, and with open source
software. Their star has faded (basically since logitech bought out the
excellent small company behind the players, slimdevices) - but a lot of
people have these around their house so the idea is to have XBMC integrate
into a bunch of these existing players for a whole house audio solution I
guess.

My add on currently includes an external headless software player, but
ideally the stream could be handed direct to XBMC's player for better
integration into XBMC (although the sync will almost certainly be less good
so the idea is to have both options).

It's all purely an exercise for me - I just use XBMC's internal music
system myself, for the most part, but others seem keen and nobody else was
doing it so I thought I'd extend my python skills a bit.

On Tue, Mar 20, 2012 at 6:22 AM, Memphiz <
reply@reply.github.com

wrote:

Confirmed, that that test file didn't play before this PR (xbmc just
crashed) and played after applying the patch. Tested on OSX and iOS.

So basically funk is flowing through my veins now :)


Reply to this email directly or view it on GitHub:
#777 (comment)

@c0diq
Copy link
Contributor

c0diq commented Mar 19, 2012

Flax and L16 are different.

-s

On Mar 19, 2012, at 3:55 PM, bossanova808 reply@reply.github.com wrote:

Basically Logitech Media Server is a music (I think it does pictures and
video now too) app from Logitech to support the Squeezebox line of network
audio players. And apparently LMS supplies streams in this format when
streaming lossless files like flac etc.

A long time ago Squeezeboxes were the most awesome network music players,
like a Sonos but a lot cheaper, more flexible, and with open source
software. Their star has faded (basically since logitech bought out the
excellent small company behind the players, slimdevices) - but a lot of
people have these around their house so the idea is to have XBMC integrate
into a bunch of these existing players for a whole house audio solution I
guess.

My add on currently includes an external headless software player, but
ideally the stream could be handed direct to XBMC's player for better
integration into XBMC (although the sync will almost certainly be less good
so the idea is to have both options).

It's all purely an exercise for me - I just use XBMC's internal music
system myself, for the most part, but others seem keen and nobody else was
doing it so I thought I'd extend my python skills a bit.

On Tue, Mar 20, 2012 at 6:22 AM, Memphiz <
reply@reply.github.com

wrote:

Confirmed, that that test file didn't play before this PR (xbmc just
crashed) and played after applying the patch. Tested on OSX and iOS.

So basically funk is flowing through my veins now :)


Reply to this email directly or view it on GitHub:
#777 (comment)


Reply to this email directly or view it on GitHub:
#777 (comment)

@bossanova808
Copy link
Contributor

Yep I know that, I believe LMS transcodes to PCM16 - the PR251 was all
about this....that guy develops a DLNA/UPNP extension to LMS

jmarshallnz added a commit that referenced this pull request Mar 20, 2012
Merged support for audio/L16 mimetype from andrewfg PR 251
@jmarshallnz jmarshallnz merged commit 7fad808 into xbmc:master Mar 20, 2012
tru pushed a commit to plexinc/plex-home-theater-public that referenced this pull request Nov 12, 2013
HolgerW1 pushed a commit to HolgerW1/xbmc that referenced this pull request Sep 26, 2014
…development

* change_postprocess_ignore_hidden_pass_nzb_delete: closes xbmcgh-803

Change PostProcessing checks and rules

+ Change replace rules and inner workings   closes xbmcgh-393
+ Include PR-620-jayme-github Process files by size but reversed (biggest first)  closes xbmcgh-620
+ Add postProcessor checks: status, quality/filesize, already processed  closes xbmcgh-629
+ Change search for airdate in database instead of thetvdb
+ Add tv download dir as default manual post-process directory  closes xbmcgh-722
+ Add is_proper to nameparser
+ Change use quality from snatch history instead of status quality
+ PEP8

Add ignore hidden, pass nzb, delete empty to processTV

Add ignore hidden subfolders (subfolder starts with .)
Removing ignored_filestrings from postProcessor
because subfolders with . are not being processed.

Add pass (inherit) nzbName to subfolder
When no videofiles are in the mainfolder and there is only one subfolder,
pass nzbName to subfolder

Add delete empty folder  closes xbmcgh-777
For now only delete empty folders on scripts and automatic.
Safety net, with manual it's easier to browse and select root folder

+ Change some logging and error messages  closes xbmcgh-702 closes xbmcgh-664 closes
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.

7 participants