Re: audio null_filter (Re: CVS commit: src/sys/dev)

[Nathanial Sloss]

G'Day,

On Sat, 29 Jul 2017 15:14:39 Tetsuya Isaki wrote:
> Thanks nat@.  It works.
> autoconfig succeeded and audioplay(1) also worked.
> Please commit it.

That's great news :)

I'll commit it and request pullups.

> 
> # Although there is still another problem that the ring buffer
> # management or somewhere is something wrong. :(
> 
> Can you describe audio(9) on cases using the special null_filter?
> 
> However, is it a correct solution to insert such an extra memcpy
> filter?  It seems to me that it's trying to cover the first bug
> with the second bug.
> 
.
For vchan_autoconfig, as you've seen there is a test for precision frequency 
and validbits the problem with vs(4) and some dec machines using a7930 audio 
codecs is that the hardware precision or the first filter in the case of vs(4) 
is equal to 4 bits and the top most filter SLINEAR is 16 bits so auto config of 
audio would fail.

By using the null filter as the first one closest to hardware with precision 
and 
validbits set to 16 -> then the acdpm filter -> then slinear auto config will 
succeed as the null filter reports 16 bits and the final slinear filter reports 
16 bits.

I would really like to do away with the null_filter and vchan_autoconfig and 
just set it to 16 bit 48000 Hz stereo.  But to do that drivers/cards that 
don't support this would have to be changed to use 
auconv/auconv_set_converter.

Ideally all sound drivers should use auconv as vchan_autoconfig is a hack.

Best regards,

Nat

> According to audio(9), we should pass the destination encoding
> (ENCODING_ADPCM) as msm6258_slinear16_to_adpcm filter's parameter,
> but msm6258_slinear16_to_adpcm filter seems to interrupt this
> as source encoding (That is why I commited the diagnostic code
> in sys/dev/ic/msm6258.c r1.18).  Therefore, I don't know details
> but I can imagine that it works by inserting this null_filter.
> 
> Thanks,
> ---
> Tetsuya Isaki 
> 

audio null_filter (Re: CVS commit: src/sys/dev)

[Tetsuya Isaki]

Thanks nat@.  It works.
autoconfig succeeded and audioplay(1) also worked.
Please commit it.

# Although there is still another problem that the ring buffer
# management or somewhere is something wrong. :(

Can you describe audio(9) on cases using the special null_filter?

However, is it a correct solution to insert such an extra memcpy
filter?  It seems to me that it's trying to cover the first bug
with the second bug.

According to audio(9), we should pass the destination encoding
(ENCODING_ADPCM) as msm6258_slinear16_to_adpcm filter's parameter,
but msm6258_slinear16_to_adpcm filter seems to interrupt this
as source encoding (That is why I commited the diagnostic code
in sys/dev/ic/msm6258.c r1.18).  Therefore, I don't know details
but I can imagine that it works by inserting this null_filter.

Thanks,
---
Tetsuya Isaki 

At Fri, 28 Jul 2017 09:58:37 +1000,
Nathanial Sloss wrote:
> Hi,
> 
> Please try the attached patch for vs(4) - It should auto configure now.
> 
> You'll need current sources from 20170728 with the changes to auconv with 
> null_filter.
> 
> NetBSD-8 is yet to be pulled up.
> 
> If the patch works - I'll commit it.
> 
> Best regards,
> 
> Nat
> 
> On Sun, 9 Jul 2017 22:52:46 Tetsuya Isaki wrote:
> > At Sun, 25 Jun 2017 02:47:28 +,
> > 
> > > Module Name:  src
> > > Committed By: nat
> > > Date: Sun Jun 25 02:47:28 UTC 2017
> > > 
> > > Modified Files:
> > >   src/sys/dev: audio.c
> > > 
> > > Log Message:
> > > Check validbits against precision in vchan_autoconfig.  At present
> > > validbits != precision is not supported.
> > > 
> > > This change will most likely break autoconfig on vs(4), for these
> > > machines the parameters can be set to the paramaters reported at attach
> > > time via sysctl.
> > 
> > Do you have any plan to revive it?
> > 
> > Thanks,
> > ---
> > Tetsuya Isaki 
> [2 vs.c.diff ]
> Index: vs.c
> ===
> RCS file: /cvsroot/src/sys/arch/x68k/dev/vs.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 vs.c
> --- vs.c  9 Jul 2017 12:49:26 -   1.39
> +++ vs.c  27 Jul 2017 23:45:17 -
> @@ -160,6 +160,8 @@ struct {
>  
>  #define NUM_RATE (sizeof(vs_l2r)/sizeof(vs_l2r[0]))
>  
> +extern stream_filter_factory_t null_filter;
> +
>  static int
>  vs_match(device_t parent, cfdata_t cf, void *aux)
>  {
> @@ -405,6 +407,12 @@ vs_set_params(void *hdl, int setmode, in
>   pfil->prepend(pfil, msm6258_slinear16_to_adpcm, play);
>   rfil->prepend(rfil, msm6258_adpcm_to_slinear16, play);
>  
> + play->validbits = 16;
> + play->precision = 16;
> +
> + pfil->prepend(pfil, null_filter, play);
> + rfil->prepend(rfil, null_filter, play);
> +
>   sc->sc_current.prate = rate;
>   sc->sc_current.rrate = rate;
>