[vlc-devel] [PATCH] auhal: Use encoded audio output if this was stored before

[Thomas Guillem]

On Mon, Feb 17, 2020, at 19:40, David Fuhrmann wrote:
> Hi Thomas,
> 
> Thanks for your feedback.
> 
>> Am 17.02.2020 um 11:47 schrieb Thomas Guillem <thomas at gllm.fr>:
>> 
>> Hello David,
>> 
>> Sorry, I have missed your ping 17 months ago on trac...
>> 
>> The patch is OK but could be improved like that:
>> 
>> diff --git a/modules/audio_output/auhal.c b/modules/audio_output/auhal.c
>> index b56e609bc39..a19aac1b505 100644
>> --- a/modules/audio_output/auhal.c
>> +++ b/modules/audio_output/auhal.c
>> @@ -519,9 +519,6 @@ RebuildDeviceList(audio_output_t * p_aout, UInt32 *p_id_exists)
>>  continue;
>>  }
>> 
>> - if (p_id_exists && i_id == i_id_exists)
>> - *p_id_exists = i_id;
>> -
>>  ReportDevice(p_aout, i_id, psz_name);
>>  CFNumberRef deviceNumber = CFNumberCreate(kCFAllocatorDefault,
>>  kCFNumberSInt32Type, &i_id);
>> @@ -542,6 +539,9 @@ RebuildDeviceList(audio_output_t * p_aout, UInt32 *p_id_exists)
>>  free(psz_encoded_name);
>>  }
>> 
>> + if (p_id_exists && i_id == i_id_exists)
>> + *p_id_exists = i_id;
>> +
> 
> 
> I think your proposal will not work: In the if clause checking for digital output, we modify i_id by adding the SPDIF flag. We really need two dedicated checks, the first one to ensure to confirm the selection of a potential analog device, and the second check is needed to confirm the selection of potentially the same device, but in the digital mode. In the end both have different IDs, so they need to be handled like two different devices.

OK, got it. Maybe a comment could be helpful then.

> 
>>  // TODO: only register once for each device
>>  ManageAudioStreamsCallback(p_aout, p_devices[i], true);
>> 
>> 
>> On Sat, Feb 15, 2020, at 12:37, david.fuhrmann at gmail.com wrote:
>>> From: David Fuhrmann <dfuhrmann at videolan.org>
>>> 
>>> Encoded output is stored in VLCs settings by adding the
>>> AOUT_VAR_SPDIF_FLAG flag to the integer. Make sure this
>>> configuration is also picked up again after restart, if it is stored
>>> and actually available.
>>> 
>>> Credits: Andrey Y.
>>> fixes #21170
>>> ---
>>> 
>>> Hello all,
>>> 
>>> Patch is taken from the user comment in #21170 and has been 
>>> successfully tested and
>>> verified by me again.
>>> I asked the user in the forum (where he posted the same patch again) if 
>>> we wants to send a git-patch to get correct authorship.
>>> As he did not respond and I do not know his full name, I would apply it
>>> as proposed, they are no objections.
>>> 
>>> This should be also backported for 3.0.9.
>>> 
>>> BR. David
>>> 
>>> 
>>> modules/audio_output/auhal.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>> 
>>> diff --git a/modules/audio_output/auhal.c b/modules/audio_output/auhal.c
>>> index b56e609bc397..643c3fd19450 100644
>>> --- a/modules/audio_output/auhal.c
>>> +++ b/modules/audio_output/auhal.c
>>> @@ -540,6 +540,9 @@ RebuildDeviceList(audio_output_t * p_aout, UInt32 
>>> *p_id_exists)
>>>  CFArrayAppendValue(currentListOfDevices, deviceNumber);
>>>  CFRelease(deviceNumber);
>>>  free(psz_encoded_name);
>>> +
>>> + if (p_id_exists && i_id == i_id_exists)
>>> + *p_id_exists = i_id;
>>>  }
>>> 
>>>  // TODO: only register once for each device
>>> @@ -1749,6 +1752,11 @@ static int Open(vlc_object_t *obj)
>>>  {
>>>  int dev_id_int = atoi(psz_audio_device);
>>>  UInt32 dev_id = dev_id_int < 0 ? 0 : dev_id_int;
>>> +
>>> + bool isDigital = (dev_id & AOUT_VAR_SPDIF_FLAG) != 0;
>>> + msg_Dbg(obj, "Trying to use stored audio device %d (%s)",
>>> + (dev_id & ~AOUT_VAR_SPDIF_FLAG), isDigital ? "digital" 
>>> : "analog");
>> 
>> Also, this should be in a separate commit.
> 
> Sure, will split that up before merging.

Thanks!

> BR. David
> 
>> 
>> I'm OK for a backport (before the upcoming release).
>> 
>> 
>>> +
>>>  RebuildDeviceList(p_aout, &dev_id);
>>>  p_sys->i_new_selected_dev = dev_id;
>>>  free(psz_audio_device);
>>> -- 
>>> 2.21.1 (Apple Git-122.3)
>>> 
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200218/90a40ca3/attachment.html>