Anything related to PR, release planning and any other non-technical idea how to move the project forward should be discussed here.
-
cc9cii
- Posts: 523
- Joined: 28 Mar 2013, 04:01
Post
by cc9cii » 19 Feb 2014, 22:04
Sorry if I'm stating the obvious but mVideoState::AudioTrack is uninitialised which results in a null pointer when trying to get its audio clock value:
Code: Select all
// accumulate the clock difference
double diff = mVideoState->get_master_clock() - mVideoState->get_audio_clock();
mAudioDiffAccum = diff + mAudioDiffAvgCoef * mAudioDiffAccum;
if(mAudioDiffAvgCount < AUDIO_DIFF_AVG_NB)
mAudioDiffAvgCount++;
else
Looking further it is initialised here but with an empty object according to the debugger:
Code: Select all
switch(codecCtx->codec_type)
{
case AVMEDIA_TYPE_AUDIO:
this->audio_st = pFormatCtx->streams + stream_index;
decoder.reset(new MovieAudioDecoder(this));
this->AudioTrack = MWBase::Environment::get().getSoundManager()->playTrack(decoder, MWBase::SoundManager::Play_TypeMovie);
if(!this->AudioTrack)
{
avcodec_close((*this->audio_st)->codec);
this->audio_st = NULL;
return -1;
}
break;
So we don't seem to have a SoundManager initialised at this point, i.e. stream_open().
-
cc9cii
- Posts: 523
- Joined: 28 Mar 2013, 04:01
Post
by cc9cii » 19 Feb 2014, 23:43
EDIT: scratch crazy comment
Anyway, not sure why the code is in the audio codec part of the switch statement. Aren't we playing a video?
Last edited by
cc9cii on 20 Feb 2014, 00:01, edited 1 time in total.
-
scrawl
- Posts: 2152
- Joined: 18 Feb 2012, 11:51
Post
by scrawl » 20 Feb 2014, 00:01
This analysis is not quite correct.
The problem is that since corristo's change, SoundManager::playTrack will actually decode samples, which means the virtual Sound_Decoder::read method will be called. And for the video, that is a MovieAudioDecoder::read which calls get_audio_clock(), and that depends on the AudioTrack pointer already being set up. However, since we haven't even returned from the playTrack method yet, the pointer is not set.
-
cc9cii
- Posts: 523
- Joined: 28 Mar 2013, 04:01
Post
by cc9cii » 20 Feb 2014, 00:03

Thanks scrawl for setting me straight. I was just poking around and thinking out loud so it wasn't really a considered analysis as such.

Last edited by
cc9cii on 20 Feb 2014, 00:06, edited 1 time in total.
-
scrawl
- Posts: 2152
- Joined: 18 Feb 2012, 11:51
Post
by scrawl » 20 Feb 2014, 00:06
edit: ok
Last edited by
scrawl on 20 Feb 2014, 00:21, edited 1 time in total.
-
cc9cii
- Posts: 523
- Joined: 28 Mar 2013, 04:01
Post
by cc9cii » 20 Feb 2014, 00:07
I was typing as fast as I could to correct it but it was still too late!
-
Chris
- Posts: 1586
- Joined: 04 Sep 2011, 08:33
Post
by Chris » 20 Feb 2014, 00:14
Well crap, I missed that when aiding corristo.
It may be enough to just revert commit 51fb9f65ea5086433fa0d1db12197b133a9b27fd so that it decodes the initial batch in the decode thread, rather than on play. I rather dislike first-time-only special cases, but that seems to be the way to go here for now.
-
gus
- Posts: 390
- Joined: 11 Aug 2011, 15:41
Post
by gus » 20 Feb 2014, 11:15
I think I got the culprit. In ESM writter, line 85,86,
Code: Select all
writeT<int>(0); // Size goes here
writeT<int>(0); // Unused header?
should be
Code: Select all
writeT<uint32_t>(0); // Size goes here
writeT<uint32_t>(0); // Unused header?
it seems to work with this change.
-
Zini
- Posts: 5538
- Joined: 06 Aug 2011, 15:16
Post
by Zini » 20 Feb 2014, 12:35
Thanks. Applying fix now. Should have checked this part first. But then, this should also cause reloading content files saved with OpenCS to break on Windows. I guess no one ever tried that.
-
Zini
- Posts: 5538
- Joined: 06 Aug 2011, 15:16
Post
by Zini » 20 Feb 2014, 12:38
The version handling fix is kinda annoying. Now I can't update the version number (which I normally do shortly after creating the release branch) without adding a tag.