AE: make sure sink suppports required samplerate for AC3 and DTS #4338

Merged
merged 2 commits into from Mar 8, 2014

Conversation

Projects
None yet
6 participants
@FernetMenta
Member

FernetMenta commented Mar 6, 2014

Output samplerates for DTS and AC3 maybe 44.1khz, or 48khz, or something else. We need to make sure that the sink does support required sample rate before opening passthrough mode.

@FernetMenta FernetMenta added the Gotham label Mar 6, 2014

@t-nelson

This comment has been minimized.

Show comment
Hide comment
@t-nelson

t-nelson Mar 6, 2014

Contributor

Confirmed

jenkins build this please

Contributor

t-nelson commented Mar 6, 2014

Confirmed

jenkins build this please

@t-nelson

This comment has been minimized.

Show comment
Hide comment
@t-nelson

t-nelson Mar 7, 2014

Contributor

Needs some OMXPlayer love.

Contributor

t-nelson commented Mar 7, 2014

Needs some OMXPlayer love.

@@ -511,12 +511,12 @@ AEDataFormat OMXPlayerAudio::GetDataFormat(CDVDStreamInfo hints)
/* check our audio capabilties */
/* pathrought is overriding hw decode*/
- if(hints.codec == AV_CODEC_ID_AC3 && CAEFactory::SupportsRaw(AE_FMT_AC3) && !CSettings::Get().GetBool("audiooutput.dualaudio"))
+ if(hints.codec == AV_CODEC_ID_AC3 && CAEFactory::SupportsRaw(AE_FMT_AC3, hints.samplerate) && !CSettings::Get().GetBool("audiooutput.dualaudio"))

This comment has been minimized.

@FernetMenta

FernetMenta Mar 7, 2014

Member

@popcornmix could you have a look at this. the PI sink pushes the supported samplerates to the list, right?

@FernetMenta

FernetMenta Mar 7, 2014

Member

@popcornmix could you have a look at this. the PI sink pushes the supported samplerates to the list, right?

This comment has been minimized.

@popcornmix

popcornmix Mar 7, 2014

Member

No. Do I just want to push formats supported by passthrough?
Technically I'd like all sample rate to be supported for PCM (the analogue output is driven by a PLL, and is genuinely has a continuous range).

@popcornmix

popcornmix Mar 7, 2014

Member

No. Do I just want to push formats supported by passthrough?
Technically I'd like all sample rate to be supported for PCM (the analogue output is driven by a PLL, and is genuinely has a continuous range).

This comment has been minimized.

@popcornmix

popcornmix Mar 7, 2014

Member

Can you cherry-pick
popcornmix/xbmc@b945a0e
if it looks okay.

@popcornmix

popcornmix Mar 7, 2014

Member

Can you cherry-pick
popcornmix/xbmc@b945a0e
if it looks okay.

This comment has been minimized.

@FernetMenta

FernetMenta Mar 7, 2014

Member

https://github.com/xbmc/xbmc/blob/master/xbmc/cores/AudioEngine/Sinks/AESinkPi.cpp#L285

I see you push 48khz AC3, DTS, EAC3. If the sink supports i.e. AC3 with 44.1khz, you should to push 44.1 too.

@FernetMenta

FernetMenta Mar 7, 2014

Member

https://github.com/xbmc/xbmc/blob/master/xbmc/cores/AudioEngine/Sinks/AESinkPi.cpp#L285

I see you push 48khz AC3, DTS, EAC3. If the sink supports i.e. AC3 with 44.1khz, you should to push 44.1 too.

@FernetMenta

This comment has been minimized.

Show comment
Hide comment
@FernetMenta

FernetMenta Mar 7, 2014

Member

jenkins build this please

Member

FernetMenta commented Mar 7, 2014

jenkins build this please

+ {
+ AESampleRateList::iterator itt4;
+ itt4 = find(info.m_sampleRates.begin(), info.m_sampleRates.end(), samplerate);
+ if (itt4 != info.m_sampleRates.end())

This comment has been minimized.

@Jalle19

Jalle19 Mar 7, 2014

Member

This could be changed to a one-liner.

@Jalle19

Jalle19 Mar 7, 2014

Member

This could be changed to a one-liner.

@FernetMenta

This comment has been minimized.

Show comment
Hide comment
@FernetMenta

FernetMenta Mar 7, 2014

Member

jenkins build this please

Member

FernetMenta commented Mar 7, 2014

jenkins build this please

@KeyserSoze1

This comment has been minimized.

Show comment
Hide comment
@KeyserSoze1

KeyserSoze1 Mar 8, 2014

Contributor

Should CAEDeviceInfo be re-structured so that sample rate/sample sizes are associated with a format. This way you could report 44.1 for PCM but not AC3 if for some reason your device did not support it.

Contributor

KeyserSoze1 commented Mar 8, 2014

Should CAEDeviceInfo be re-structured so that sample rate/sample sizes are associated with a format. This way you could report 44.1 for PCM but not AC3 if for some reason your device did not support it.

@FernetMenta

This comment has been minimized.

Show comment
Hide comment
@FernetMenta

FernetMenta Mar 8, 2014

Member

If "some reason" is an actual case, we should consider this for G+1. For Gotham the risk is too high that something breaks,

Member

FernetMenta commented Mar 8, 2014

If "some reason" is an actual case, we should consider this for G+1. For Gotham the risk is too high that something breaks,

@FernetMenta

This comment has been minimized.

Show comment
Hide comment
@FernetMenta

FernetMenta Mar 8, 2014

Member

jenkins build this please

Member

FernetMenta commented Mar 8, 2014

jenkins build this please

@KeyserSoze1

This comment has been minimized.

Show comment
Hide comment
@KeyserSoze1

KeyserSoze1 Mar 8, 2014

Contributor

Well I ask because for example, HDMI EDID could report 44.1 for PCM and not AC3 for example, now are there any devices that actually have this exact case I do not know. But the information is structured so it could. Just suggesting. And I do not argue with waiting till after Gotham for the exact reason you mention.

Contributor

KeyserSoze1 commented Mar 8, 2014

Well I ask because for example, HDMI EDID could report 44.1 for PCM and not AC3 for example, now are there any devices that actually have this exact case I do not know. But the information is structured so it could. Just suggesting. And I do not argue with waiting till after Gotham for the exact reason you mention.

@FernetMenta

This comment has been minimized.

Show comment
Hide comment
@FernetMenta

FernetMenta Mar 8, 2014

Member

Please don't hesitate to remind us or pr some code with your ideas after release. There is lot of room for improvement in this area.

Member

FernetMenta commented Mar 8, 2014

Please don't hesitate to remind us or pr some code with your ideas after release. There is lot of room for improvement in this area.

jmarshallnz added a commit that referenced this pull request Mar 8, 2014

Merge pull request #4338 from FernetMenta/google
AE: make sure sink suppports required samplerate for AC3 and DTS

@jmarshallnz jmarshallnz merged commit b11df8b into xbmc:master Mar 8, 2014

1 check passed

default Merged build #331 succeeded in 2 hr 41 min
Details

jmarshallnz added a commit that referenced this pull request Mar 9, 2014

Merge pull request #4338 from FernetMenta/google
AE: make sure sink suppports required samplerate for AC3 and DTS

@jmarshallnz jmarshallnz removed the Gotham label Mar 9, 2014

@FernetMenta FernetMenta deleted the FernetMenta:google branch Mar 9, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment