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

Alexandre Janniaux ajanni at videolabs.io
Fri Oct 25 14:45:16 CEST 2019


Hi,

To be honest, when the main answer is "it was decided
decades ago", I do feel it's a great secret wisdom.

But if the answer is just that it is useless, yes there is
probably no need to invoke decades of inactive contributors
opinion, hard-to-find discussion or experienced busy
developers.

Thomas's patch seems to make sense though, as a lot of
audio softwares are implementing it. There are also
probably some different use cases for exclusive mode when
using VLC in a different use case than the home multimedia
player.

Also, like Thomas mentioned, it's not about adding 24bit
input support so it actually doesn't even contradict the
decision that was taken.

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, Oct 25, 2019 at 03:26:50PM +0300, Rémi Denis-Courmont wrote:
> Hi,
>
> I wonder what kind of great secret wisdom you expect was behind the decision to drop everything except double, float, S32, S16 and U8. We obviously need float and S16, and nobody dared to remove the other three. I'm pretty sure we could remove U8.
>
> It's just that everything else is completely useless, including all 12 variants of 24-bits PCM.
>
> Le 25 octobre 2019 14:33:08 GMT+03:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
> >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
> >_______________________________________________
> >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


More information about the vlc-devel mailing list