[mythtv] Audio Update - Another patch

[Jesper Sörensen]

Ed Wildgoose wrote:

>
>> Thanks! Getting some problems here:
>
>
>
> Ooops, yes.  Spot on.  Forgot that file.  See attached patch:


Great! I'm not able to do much more than compile testing right now 
though since I don't have access to the system at the moment.

>
>> +    if (pcm_handle == NULL)
>> +    {
>> +        VERBOSE(VB_IMPORTANT, QString("WriteAudio() called with 
>> pcm_handle == NULL!"));
>> +        return;
>> +    }
>> +
>>  
>>
>
> Is this stuff really needed now if we never close the device?  No 
> point having redundant code cluttering up the place?  How would we 
> ever have a null handle now?


Well, good question! I fixed an other Alsa crash a couple of months ago 
(same cause - Null device), so I figured I'd do it properly and future 
safe now... :-)

In theory it should't be needed, but I've only looked at 
audiooutputalsa.cpp so I don't know how/when the Alsa functions are 
used? It seemed to me like NVP didn't know or care if the device was 
open or closed - it wanted to use all the sound functions anyway... I 
also don't know how the audio threading works so I figured better safe 
than sorry.

On the other hand, it *does* clutter up the code and all it really does 
is fails more gracefully (I.E. you get no sound and lots of errors in 
the log instead of abending the whole program). If you decide to remove 
the warning you should probably remove the checks completely so it 
crashes, since failing silently is not good IMHO. I'll let someone who 
knows more about Myth make that decision though.

>
>> +            // CloseDevice();
>>  
>>
>
> Why do we need this comment?  Can we remove this line now?


As you know, there previously was a snd_pcm_close() call there. I 
figured we didn't need it (IMHO closing the device is bad), but if there 
really is a reason/need for the device to sometimes be closed it should 
at least be done via the CloseDevice method to reduce redundancy 
(abstraction is good and that method also has the proper Null check).

I left that comment there as a reminder for anyone who might want to 
reinsert the device-killing code to do it properly, but if you and David 
George (I don't know who else might have been involved in the Alsa 
code?) agree that this approach is better and no killing is needed then 
it can of course be removed.

Feel free to modify my patch any way you want. If you don't remove it 
now we can always remove it later when we know that everything works...

Thanks again for all your work, and let's hope it gets commited to CVS 
soon! 8-)

Jesper