[Feature/RFC] Support external audio tracks with dvdplayer #2562

ace20022 · 2013-04-07T21:28:39Z

This PR adds the possibility to play external audio tracks together with video files.
The original PR is here:#1337. Currently there is no support for language/type extraction from the filename. It can easily be added after #1365 got merged, the code for that is in my repo.

I implemented it with minimal code changes/additions in mind. The idea is to drop/ignore audio packets from internal tracks if an external track is selected. The read rate is hard-coded at the moment, maybe someone has a solution for that flaw. I'm not sure if this leads to performance issues on low power devices.
There are at least 2 shortcomings at the moment:

playback stops if the selected external track reaches its end (before the end of the video stream)
seeking may lead to temporary stutter

Two other approaches that came to my mind are

allow more than one player at a time and mix or prioritize the streams
handle/decode video and audio frames separately, i.e., extend this idea: Karaoke video background player with a generic decoder class #1137

BTW this was my first contact with c++ and xbmc-code, so it's certainly far from perfect.

MartijnKaijser · 2013-04-08T11:45:33Z

@cptspiff
as you requested

akva2 · 2013-04-08T11:47:20Z

thanks, seen it but i also have this pesky thing called a real life job ;)

ace20022 · 2013-04-09T21:25:36Z

@cptspiff

i'm wondering if zipped/rarred extra audio tracks is common enough to warrant the overhead...

Not sure either, just copied it from the external subtitle code. I think I should refactor and reuse that.

some of this code made me deja vu ;)

I will think about a way to remove the duplications :)

Thanks a lot so far! I'm afraid it will take some time to update it; I simply don't have much spare time at the moment ;(

ace20022 · 2013-04-14T18:31:46Z

@cptspiff I've implemented most of your comments. The overall diff of util.c is quite a mess due to the renaming I've done. I wasn't aware of that because I only saw the individual diffs until I've push them to here. I can revert the renaming stuff if you prefer that.

Things not done yet:

I've commented to the first one: ace20022@9d3848b#xbmc-cores-dvdplayer-dvdplayer-cpp-P172
The code duplication in DVDPlayer and DVDFileInfo.
- Do you think I can solve this by adding an new class?
playback stops if the selected external track reaches its end (before the end of the video stream)
- maybe fall back to next audio stream?
seeking may lead to temporary stutter
- really no glue :(

Best regards!

elupus · 2013-04-14T19:39:14Z

This get a bit messy in dvdplayer. The code need to be generalized so external and internal demuxers are handled similar. Making the m_pInputStream and m_pDemuxer arrays them selfes, so we can iterate over them easily and use the same code independent on where it is reading from.

ace20022 · 2013-04-15T07:50:21Z

@elupus finally ;-)

ace20022 · 2013-04-25T11:35:33Z

@cptspiff are the utils changes okay? If so I could clean this pr up a bit.

akva2 · 2013-04-25T11:42:11Z

no time to review in detail now, but in principle they look exactly what i had in mind.

ace20022 · 2013-04-26T12:56:55Z

@elupus I've uploaded an approach of your suggestion to a separate branch: https://github.com/ace20022/xbmc/commits/ext_audio_unified (https://github.com/ace20022/xbmc/compare/ext_audio_unified)
Of course, only the changes to dvdplayer.c/h are of interest. Could you please have look at it when you find some time?

ace20022 · 2013-05-07T12:52:32Z

@elupus @cptspiff Could you please have a look at the above-mentioned commit. If this is the way to go, I can revise this pr. (I'm a bit bored by all the cppcheck stuff by now ;) )

jenkins4xbmc · 2013-06-16T19:42:41Z

Is this PR ment to be tested by jenkins?

ace20022 · 2013-06-29T09:40:48Z

@elupus @cptspiff New attempt, are you still interested?
Rebased and implemented elupus' suggestion, at least if I understood it correctly ;)

xbmc/cores/dvdplayer/DVDPlayer.cpp

-    m_pInputStream->Abort();
+  for(unsigned int i = 0; i < m_pInputStreams.size(); i++)
+  {
+    if(m_pInputStreams[i])


ace20022 · 2013-07-04T11:44:37Z

@elupus Thanks for the review. I've made some changes, could you please have another look?

gladeo · 2013-07-30T12:09:19Z

Hi,
i would like to play together m2v and wav files (from camcorder) in xbmc

my cam saves like this:
video.m2v
audio_1.wav (left channel)
audio_2.wav (right channel)

i have tons of such material and i don't want to put all that stauff in container or use any conversion

Thank you

MartijnKaijser · 2013-08-07T06:13:33Z

@elupus
can you look over this PR?

ace20022 · 2013-08-09T19:37:17Z

@elupus I have cleaned up the mess regarding the not squashed commits. Sorry again. Could you please review it again?

NanNans · 2013-08-16T05:17:06Z

Hello! Sorry to drop in like this with a non-technical message, but I just wanted to let you know @ace20022 that I am really excited by the possibilities of your work here, and I really appreciate all the time you've spent on this. Just by jumping around the various forum posts about this project I've gotten a sense of what a long and frustrating journey it has been to get to this point. The support for external audio tracks seems like such an obviously needed feature (I'm sure that the kind of people who use XBMC are the kind of people who like listening to DVD commentaries) that I'm surprised you've had to do so much on your own. I wish I could offer some useful help, but my coding skills are nowhere near strong enough. The best I can do is cheer you on from the sidelines and let you know that your work is not in vain. I can hardly wait for the chance to try out what you've been doing... my commentary tracks have been silent for too long! Thank you!

ace20022 · 2013-09-07T20:40:46Z

@elupus ping.

…500ms and log error case

[depends] - add missing dependency for python native (needs libz-native)

Aefixes

[darwinsink] - increase the initial startup timeout of the sinks to 1900...

[pvr] fixes related to chanel icons

[shairplay] - upstream backports - fix password protection and socket race condition

Add void CUtil::GetPathsToLookForAssociatedItems() Add void CUtil::ScanPathsForAssociatedItems(...) Add int CUtil::ScanArchiveForAssociatedItems()

ScanForExternalSubtitles(...) now uses GetPathsToLookForAssociatedItems() and ScanPathsForAssociatedItems(...). Remove obsolete method ScanArchiveForSubtitles() and sub_exts[].

…store them.

… them. Update the SelectionStream. Adapt all relevant places where m_pDemuxer is used; in particular ReadPacket(...) and add ReadExternalAudioPacket(...)

…store them.

… them. Update the SelectionStream. Adapt all relevant places where m_pDemuxer is used; in particular ReadPacket(...) and add ReadExternalAudioPacket(...)

…rnal audio files (e.g. language and channels).

ghost self-assigned this Apr 8, 2013

elupus reviewed Jun 29, 2013
View reviewed changes

xbmc/cores/dvdplayer/DVDPlayer.cpp

m_pInputStream->Abort();

for(unsigned int i = 0; i < m_pInputStreams.size(); i++)

{

if(m_pInputStreams[i])

This comment was marked as spam.

Sign in to view

Memphiz and others added 16 commits March 25, 2014 17:40

[darwinsink] - increase the initial startup timeout of the sinks to 4…

efd3fff

…500ms and log error case

Merge pull request xbmc#4466 from Memphiz/dependsfix

7792345

[depends] - add missing dependency for python native (needs libz-native)

Merge pull request xbmc#4472 from FernetMenta/aefixes

ee35ba9

Aefixes

Merge pull request xbmc#4471 from Memphiz/sinktimeout

00ea74f

[darwinsink] - increase the initial startup timeout of the sinks to 1900...

Merge pull request xbmc#4477 from xhaggi/pvr-icon-fixes

840b7d9

[pvr] fixes related to chanel icons

Merge pull request xbmc#4476 from Memphiz/shairplayfix

1bb64ef

[shairplay] - upstream backports - fix password protection and socket race condition

[Util] Generalize external subtitle retrieval method(s).

af65f65

Add void CUtil::GetPathsToLookForAssociatedItems() Add void CUtil::ScanPathsForAssociatedItems(...) Add int CUtil::ScanArchiveForAssociatedItems()

[Util] Make use of generalized ScanForExternalSubtitles(...) methods.

4afb2ec

ScanForExternalSubtitles(...) now uses GetPathsToLookForAssociatedItems() and ScanPathsForAssociatedItems(...). Remove obsolete method ScanArchiveForSubtitles() and sub_exts[].

[Util] Add method to scan for external audio files.

9be5e5d

[DVDPlayer] Open input streams for internal and ext. audio files and …

39f1902

…store them.

[DVDPlayer] Add functionality to open external demuxers and read from…

0885a08

… them. Update the SelectionStream. Adapt all relevant places where m_pDemuxer is used; in particular ReadPacket(...) and add ReadExternalAudioPacket(...)

[DVDPlayer] Handle bogus/erroneous ext. audio files.

3632fb4

[OMXPlayer] Open input streams for internal and ext. audio files and …

e86fddf

…store them.

[OMXPlayer] Add functionality to open external demuxers and read from…

b3f3fd9

… them. Update the SelectionStream. Adapt all relevant places where m_pDemuxer is used; in particular ReadPacket(...) and add ReadExternalAudioPacket(...)

[OMXPlayer] Handle bogus/erroneous ext. audio files

436d499

[DVDFileInfo] Adapt DVDFileInfo so that it reads infos also from exte…

e3bbddc

…rnal audio files (e.g. language and channels).

ace20022 reopened this Mar 28, 2014

t-nelson added the The Hoff label Apr 3, 2014

ace20022 removed this from the Helix 14.0-alpha1 milestone Apr 28, 2014

MartijnKaijser added this to the Pending for inclusion milestone May 21, 2014

ace20022 removed the Helix label Sep 1, 2014

ace20022 mentioned this pull request Sep 8, 2014

dvdplayer: do a seek after having changed subtitle stream #5312

Closed

This was referenced Jan 2, 2015

[Util] Generalize external subtitle retrieval method(s). #6096

Closed

[Feature] Support external audio tracks (part III) #6104

Closed

ace20022 closed this Jan 15, 2015

MartijnKaijser modified the milestones: Pending for inclusion, Abandoned, obsolete or superseeded Mar 21, 2015

ace20022 deleted the external_audio branch May 22, 2015 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature/RFC] Support external audio tracks with dvdplayer #2562

[Feature/RFC] Support external audio tracks with dvdplayer #2562

ace20022 commented Apr 7, 2013

MartijnKaijser commented Apr 8, 2013

akva2 commented Apr 8, 2013

ace20022 commented Apr 9, 2013

ace20022 commented Apr 14, 2013

elupus commented Apr 14, 2013

ace20022 commented Apr 15, 2013

ace20022 commented Apr 25, 2013

akva2 commented Apr 25, 2013

ace20022 commented Apr 26, 2013

ace20022 commented May 7, 2013

jenkins4xbmc commented Jun 16, 2013

ace20022 commented Jun 29, 2013

This comment was marked as spam.

ace20022 commented Jul 4, 2013

gladeo commented Jul 30, 2013

MartijnKaijser commented Aug 7, 2013

ace20022 commented Aug 9, 2013

NanNans commented Aug 16, 2013

ace20022 commented Sep 7, 2013

[Feature/RFC] Support external audio tracks with dvdplayer #2562

[Feature/RFC] Support external audio tracks with dvdplayer #2562

Conversation

ace20022 commented Apr 7, 2013

MartijnKaijser commented Apr 8, 2013

akva2 commented Apr 8, 2013

ace20022 commented Apr 9, 2013

ace20022 commented Apr 14, 2013

elupus commented Apr 14, 2013

ace20022 commented Apr 15, 2013

ace20022 commented Apr 25, 2013

akva2 commented Apr 25, 2013

ace20022 commented Apr 26, 2013

ace20022 commented May 7, 2013

jenkins4xbmc commented Jun 16, 2013

ace20022 commented Jun 29, 2013

This comment was marked as spam.

ace20022 commented Jul 4, 2013

gladeo commented Jul 30, 2013

MartijnKaijser commented Aug 7, 2013

ace20022 commented Aug 9, 2013

NanNans commented Aug 16, 2013

ace20022 commented Sep 7, 2013