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

No audio playback of MPEG2 files #55

Closed
JustFred51 opened this issue Nov 20, 2015 · 10 comments
Closed

No audio playback of MPEG2 files #55

JustFred51 opened this issue Nov 20, 2015 · 10 comments

Comments

@JustFred51
Copy link
Contributor

Issue was reported here:
http://forums.sagetv.com/forums/showthread.php?p=579435&postcount=11

SageTV details the file as:
MPEG2-PS[MPEG2-Video/3.8Mbps 4:3 480i @ 29.97fps, MP2/384Kbps@48kHz Stereo

The problem follows certain vintages of MpegDeMux.ax. The regression was introduced with the commit for ae41796

The problem has been traced to:
native\ax\Native2.0\NativeCore\AVFormat\MpegAudioFormat.c

Making the following change resolves the reported problem:

int ReadMpegAudioHeader( MPEG_AUDIO *pMpegAudio, const uint8_t* pStart, int Size )
{
    int Layer,i,MPGVersion, CRC_protected, BiteRateIndex, SampleRateIndex;
-   int8_t LayerCode;
+   uint8_t LayerCode;
-   const int8_t *pData;
+   const uint8_t *pData;
    uint32_t Bitrate;

I'm unsure if this change will introduce other side effects, since it's unclear why Qian changed these two locals from unsigned to signed.

@qianzhang5
Copy link
Collaborator

It should be unsigned. typo.

@Narflex
Copy link
Member

Narflex commented Nov 20, 2015

Are you going to submit a change for this Fred?

@JustFred51
Copy link
Contributor Author

Given that it's Qian's regression, I was hoping he would do the submit :^)

@Narflex
Copy link
Member

Narflex commented Nov 20, 2015

Whatever is fine...I figured you found the bug, so you should get credit
for the commit fix. :-)

Jeff Kardatzke
Sent from my Android
On Nov 20, 2015 3:09 PM, "Keith Fischer" notifications@github.com wrote:

Given that it's Qian's regression, I was hoping he would do the submit :^)


Reply to this email directly or view it on GitHub
#55 (comment).

@JustFred51
Copy link
Contributor Author

I come from a culture of "if you break it, you get to fix it". I don't feel a big need to rack up brownie points and also didn't want to step on anybody's turf. Will wait for a response from Qian.

@Hiltronix
Copy link

@JustFred51 Thank you for finding the issue. I've manually edited my cloned repo and recompiled with VS2015, and FWIW I can confirm this fixes the problem I was having with no audio on mpeg videos.

Thanks again,
Carey

@JustFred51
Copy link
Contributor Author

@Hiltronix Thanks for the feedback.
I'm pretty sure that the change between unsigned/signed introduced a problem with how sign-extension affects the comparisons.
Were you able to turn on any debug messages that gave a clue about the failure? I didn't notice anything in Native.log. Still learning what what tools are available to debug the native code. I didn't find Graphedt or GraphStudio to be particularly useful, but I'm not a multimedia guy.

@Hiltronix
Copy link

@JustFred51 No, I'm in a similar boat to you, and to be honest I'm new to debugging video issues like this. I initially figured out which filter file it was mainly through trial and error, swapping out the new for old and narrowing it down, checking playback. Since then I've started on my learning curve on this topic, using GraphStudio, turning on Sage debug and watching the "DShowGraphFilters" values in "sagetv_0.txt" and "sagetvclient_0.txt", to see what filters Sage is using.

Learning more about this has helped me select codecs for improving how Sage plays back videos other than .ts and .mpg. My biggest issue with SageTV has always been the playback of file formats other than it's own recordings. It's been such a time waster over the years (for me at least) playing with codecs, splitters, etc. that are compatible with Sage, and having to learn the ins and outs of tweaking everything to have it be my main video player and not just the recorder and playback for it's own recordings. My hope for SageTV OS is that some devs with knowledge on this topic will someday make the easy playback capability of all video types as idiot proof as Kodi/XBMC , MPC or VLC. Sorry for the rant, it's not completely off topic, it's Sage video related, but probably not the correct forum. :)

@qianzhang5
Copy link
Collaborator

I submitted the fix. Thanks @JustFred51 https://github.com/JustFred51

Qian

On Fri, Nov 20, 2015 at 3:08 PM, Keith Fischer notifications@github.com
wrote:

Given that it's Qian's regression, I was hoping he would do the submit :^)


Reply to this email directly or view it on GitHub
#55 (comment).

@JustFred51
Copy link
Contributor Author

Great! Thanks.

JREkiwi pushed a commit to JREkiwi/sagetv that referenced this issue Dec 30, 2018
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

No branches or pull requests

4 participants