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

Add support for signed 24 bit packed little endian (S24_3LE). #26

Conversation

Projects
None yet
4 participants
@cdealti
Copy link

commented Sep 11, 2016

No description provided.

@tallica

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

@jlindgren90

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

This looks like a good start, but it will have to wait till after 3.8.

Some observations:

  • FMT_S24_3LE/BE should be added to the end of the enum so as not to change the values of the names that come before.
  • audio_interlace/deinterlace() will have to be updated. Shouldn't be too difficult.
  • All the macros in this file predate the switch to C++. I'd like to convert them to templates before adding any more.
  • With recent changes in 3.8, it should be possible to detect whether packed or unpacked output is needed automatically. I would rather do that than requiring the user to figure it out.
@cdealti

This comment has been minimized.

Copy link
Author

commented Sep 14, 2016

Thanks for reviewing this.

FMT_S24_3LE/BE should be added to the end of the enum so as not to change the values of the names that come before.

Will do.

audio_interlace/deinterlace() will have to be updated. Shouldn't be too difficult.

I've checked and audio_interlace/deinterlace() are only called from the jack and ffaudio plugins. Jack only supports FMT_FLOAT but I'm not sure about ffaudio. I believe FMT_S24_3LE/BE are only useful with ALSA to match the supported formats of the DAC.
For the time being I will remove them from audio_interlace/deinterlace() given that nobody should call them with the packed format.

All the macros in this file predate the switch to C++. I'd like to convert them to templates before adding any more.

OK. Let's keep this PR lean and mean given that you plan significant changes.

@cdealti cdealti force-pushed the cdealti:pr/support_24_bit_packed branch from c17f6e7 to 615561e Sep 14, 2016

@jlindgren90

This comment has been minimized.

Copy link
Member

commented Sep 18, 2016

Should be working in master now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.