[vlc-devel] [PATCH 4/5] wasapi: add 24bits support

Alexandre Janniaux ajanni at videolabs.io
Fri Oct 25 13:33:08 CEST 2019


Hi,

It seems pretty legitimate to reconsider any decision
through the TC if no consensus is found, especially if
nobody has any clues about why this decision was made.

On original J-B's post, it was written that votes and
discussion are private. Would the reason for this refusal
or acceptance be exposed on the mailing list for
documentation purpose?

The trac is a bit empty when it comes to keyword search or
normal search about this issue.

https://trac.videolan.org/vlc/query?status=assigned&status=closed&status=new&status=reopened&keywords=~24+bit

The mailing list mainly hardly mention 24bit support but no
decision of removal.

As an inexperienced developer on audio it is difficult to
catch up on this and understand the reason behind this
decision, so I would really appreciate any hints following
this one.

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, Oct 25, 2019 at 01:02:54PM +0200, Thomas Guillem wrote:
>
> On Fri, Oct 25, 2019, at 12:47, Rémi Denis-Courmont wrote:
> > Here by people here in this decade.
>
> Then, I hereby ask the VLC's community if this old decision should be kept. Maybe I should ask the VLC-TC ?
>
> VLC will continue to be denigrated by pro users if we don't offer them some options for "high quality audio".
>
> A lot of applications are offering exclusive 24bit support. By doing a quick research I found MPV and Qobuz.
>
> Yes I know 24bit 96k is useless, cf. https://people.xiph.org/~xiphmont/demo/neil-young.html
>
> This is not a reason to refuse this patch set, we should always offer a choice to pro users. Furthermore, this patch set doesn't affect the normal behavior,
>
>
> >
> > Le 25 octobre 2019 13:35:36 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
> >>
> >> On Fri, Oct 25, 2019, at 12:17, Rémi Denis-Courmont wrote:
> >>> I disagree and this contradicts earlier decisions here.
> >>
> >> Yes that is what you said in the first reply and I asked you:
> >>
> >> Who ? when ? Do you have a trace of this decision ?
> >>
> >>>
> >>> Le 25 octobre 2019 10:16:28 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
> >>>>
> >>>> On Thu, Oct 24, 2019, at 18:29, Rémi Denis-Courmont wrote:
> >>>>> So what? Some cards don't support PCM at all, or only some really weird formats. That's why we use the OS-provided HAL.
> >>>>
> >>>> In that case, we can also add support for such format.
> >>>>
> >>>> Yes, we must use the OS-provided HAL by default. But we can still add an advanced option to enable exclusive mode. I know a lot of audiophile would like to play their 24bit 96khz directly. Even if I know that 16bit 48khz cover fully the hearing range.
> >>>>
> >>>>>
> >>>>> I don't think this belongs in VLC. We already decided to drop 24-bits PCM earlier.
> >>>>
> >>>> Who ? when ? Do you have a trace ?
> >>>>
> >>>>>
> >>>>> Le 24 octobre 2019 15:37:12 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
> >>>>>> But don't use VLC_CODEC_S24N as this fourcc is not handled by any audio
> >>>>>> filters. Instead, request VLC_CODEC_S32N to VLC and do the conversion
> >>>>>> internally.
> >>>>>>
> >>>>>> This commit is needed by the next commit since some sound cards only support
> >>>>>> 24bits. modules/audio_output/wasapi.c | 30 +++++++++++++++++++++++++++---
> >>>>>>  1 file changed, 27 insertions(+), 3 deletions(-)
> >>>>>>
> >>>>>> diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c
> >>>>>> index d6105169b8e..daa946f8afc 100644
> >>>>>> --- a/modules/audio_output/wasapi.c
> >>>>>> +++ b/modules/audio_output/wasapi.c
> >>>>>> @@ -106,6 +106,7 @@ typedef struct aout_stream_sys
> >>>>>>      unsigned block_align;
> >>>>>>      UINT64 written; /**< Frames written to the buffer */
> >>>>>>      UINT32 frames; /**< Total buffer size (frames) */
> >>>>>> +    bool s24s32; /**< Output configured as S24N, but input as S32N */
> >>>>>>  } aout_stream_sys_t;
> >>>>>>
> >>>>>>  static void ResetTimer(aout_stream_t *s)
> >>>>>> @@ -268,7 +269,28 @@ static HRESULT Play(aout_stream_t *s, block_t *block, vlc_tick_t date)
> >>>>>>
> >>>>>>          const size_t copy = frames * sys->block_align;
> >>>>>>
> >>>>>> -        memcpy(dst, block->p_buffer, copy);
> >>>>>> +        if (!sys->s24s32)
> >>>>>> +        {
> >>>>>> +            memcpy(dst, block->p_buffer, copy);
> >>>>>> +            block->p_buffer += copy;
> >>>>>> +            block->i_buffer -= copy;
> >>>>>> +        }
> >>>>>> +        else
> >>>>>> +        {
> >>>>>> +            /* Convert back S32L to S24L. The following is doing the opposite
> >>>>>> +             * of S24LDecode() from codec/araw.c */
> >>>>>> +            BYTE *end = dst + copy;
> >>>>>> +            while (dst < end)
> >>>>>> +            {
> >>>>>> +                dst[0] = block->p_buffer[1];
> >>>>>> +                dst[1] = block->p_buffer[2];
> >>>>>> +                dst[2] = block->p_buffer[3];
> >>>>>> +                dst += 3;
> >>>>>> +                block->p_buffer += 4;
> >>>>>> +                block->i_buffer -= 4;
> >>>>>> +            }
> >>>>>> +
> >>>>>> +        }
> >>>>>>          hr = IAudioRenderClient_ReleaseBuffer(render, frames, 0);
> >>>>>>          if (FAILED(hr))
> >>>>>>          {
> >>>>>> @@ -276,8 +298,6 @@ static HRESULT Play(aout_stream_t *s, block_t *block, vlc_tick_t date)
> >>>>>>              break;
> >>>>>>          }
> >>>>>>
> >>>>>> -        block->p_buffer += copy;
> >>>>>> -        block->i_buffer -= copy;
> >>>>>>          block->i_nb_samples -= frames;
> >>>>>>          sys->written += frames;
> >>>>>>          if (block->i_nb_samples == 0)
> >>>>>> @@ -512,6 +532,7 @@ static int vlc_FromWave(const WAVEFORMATEX *restrict wf,
> >>>>>>              switch (wf->wBitsPerSample)
> >>>>>>              {
> >>>>>>                  case 32:
> >>>>>> +                case 24:
> >>>>>>                      audio->i_format = VLC_CODEC_S32N;
> >>>>>>                      break;
> >>>>>>                  case 16:
> >>>>>> @@ -679,6 +700,9 @@ static HRESULT Start(aout_stream_t *s, audio_sample_format_t *restrict pfmt,
> >>>>>>      sys->format = fmt.i_format;
> >>>>>>      sys->block_align = pwf->nBlockAlign;
> >>>>>>      sys->rate = pwf->nSamplesPerSec;
> >>>>>> +    sys->s24s32 = pwf->wBitsPerSample == 24 && fmt.i_format == VLC_CODEC_S32N;
> >>>>>> +    if (sys->s24s32)
> >>>>>> +        msg_Dbg(s, "audio device configured as s24");
> >>>>>>
> >>>>>>      hr = IAudioClient_Initialize(sys->client, shared_mode, 0, buffer_duration,
> >>>>>>                                   0, pwf, sid);
> >>>>>
> >>>>> --
> >>>>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> >>>>> _______________________________________________
> >>>>> vlc-devel mailing list
> >>>>> To unsubscribe or modify your subscription options:
> >>>>> https://mailman.videolan.org/listinfo/vlc-devel
> >>>>
> >>>
> >>> --
> >>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> >>> _______________________________________________
> >>> vlc-devel mailing list
> >>> To unsubscribe or modify your subscription options:
> >>> https://mailman.videolan.org/listinfo/vlc-devel
> >>
> >
> > --
> > Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> > _______________________________________________
> > 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


More information about the vlc-devel mailing list