From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Cc: "Hans Verkuil" <hverkuil@xs4all.nl>, "r.verdejo@samsung.com" <r.verdejo@samsung.com>, "nicolas@ndufresne.ca" <nicolas@ndufresne.ca>, "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>, "skhan@linuxfoundation.org" <skhan@linuxfoundation.org>, "linux-kernel-mentees@lists.linuxfoundation.org" <linux-kernel-mentees@lists.linuxfoundation.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [v10 0/4] media: vidtv: Implement a virtual DVB driver Date: Sat, 12 Sep 2020 11:49:01 -0300 [thread overview] Message-ID: <15028D87-A496-4CAD-91A6-E4467489C4D0@getmailspring.com> (raw) In-Reply-To: <20200912111506.2d2bd512@coco.lan> Hi Hans and Mauro & all >Why the dvb_ prefix? All virtual drivers just start with 'vi'. > >And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko? >Just like the other virtual media drivers? I guess Mauro was the one to come up with the dvb_* prefix for the kernel modules for the reasons he explicited up in this thread. As far as dvb_vidtv_bridge.ko and vidtv_bridge.c, I just wanted to be verbose so that people would look at this and see that it is the code for a bridge driver, since this is also supposed to be a template. Also because I had some trouble myself figuring out what was what when first browsing through other dvb drivers. That said, I am 100% onboard with renaming this to vidtv.ko or whatever seems more appropiate :) >Yet, I agree with you that at least an alias is needed. >earlier today, I wrote a patch with such purpose: If you all would like to just leave this at that ^ I am also ok with it. >For regression testing of vidtv during the daily build it would be great if >the contrib/test/test-media script can be enhanced to include vidtv. Sure, I can do that if you'd like. Can you provide some tips on how to get started? :) >Note that this script relies on the /dev/mediaX devices to run the tests. I >noticed that vidtv doesn't appear to create a /dev/mediaX device, even though >CONFIG_MEDIA_CONTROLLER_DVB=y. This is definitely something that would be good >to support in vidtv. I didn't write any code for this specifically. I was under the impression that the dvb core would create any/all the necessary files. Mauro? >I'm thinking on something like: > >echo 1 >/sys/kernel/debug/vidtv/discontinuity > >to generate a frame numbering discontinuity > >and other things like that, changing S/N ratio and other parameters >and injecting errors, in order to test the effects on user apps. Can you provide an initial list of things you would like to see? I can then implement these and we can go from there When you say 'frame numbering discontinuity', do you mean having a gap in the the TS continuity counter for a given pid? For the s302m encoder, maybe we can add some noise to the samples? > changing S/N ratio Btw 'lose lock if signal goes bad' code in vidtv_tuner.c and vidtv_demod.c does not work currently. Mainly because they expect an array of valid frequencies to compare to and as of now there's no default. I mean these, just to be clear: static unsigned int vidtv_valid_dvb_t_freqs[NUM_VALID_TUNER_FREQS]; module_param_array(vidtv_valid_dvb_t_freqs, uint, NULL, 0); MODULE_PARM_DESC(vidtv_valid_dvb_t_freqs, "Valid DVB-T frequencies to simulate"); static unsigned int vidtv_valid_dvb_c_freqs[NUM_VALID_TUNER_FREQS]; module_param_array(vidtv_valid_dvb_c_freqs, uint, NULL, 0); MODULE_PARM_DESC(vidtv_valid_dvb_c_freqs, "Valid DVB-C frequencies to simulate"); static unsigned int vidtv_valid_dvb_s_freqs[NUM_VALID_TUNER_FREQS]; module_param_array(vidtv_valid_dvb_s_freqs, uint, NULL, 0); MODULE_PARM_DESC(vidtv_valid_dvb_s_freqs, "Valid DVB-C frequencies to simulate"); static unsigned int max_frequency_shift_hz; module_param(max_frequency_shift_hz, uint, 0); MODULE_PARM_DESC(max_frequency_shift_hz, "Maximum shift in HZ allowed when tuning in a channel") > No. I meant the audio was not a sinusoidal wave after such change. This encoder is dead simple and yet I have been struggling :/ Here is the relevant decoding code in libavcodec/s302m.c: for (; buf_size > 4; buf_size -= 5) { *o++ = (ff_reverse[buf[1]] << 8) | ff_reverse[buf[0]]; *o++ = (ff_reverse[buf[4] & 0xf0] << 12) | (ff_reverse[buf[3]] << 4) | (ff_reverse[buf[2]] >> 4); buf += 5; } I am somewhat confident that the opposite to the above is precisely what I sent you last: > f.data[0] = reverse[sample & 0xff]; > f.data[1] = reverse[(sample & 0xff00) >> 8]; > f.data[2] = (vucf << 4) | (reverse[(sample & 0x0f)] >> 4); > f.data[3] = reverse[(sample & 0x0ff0) >> 4]; > f.data[4] = reverse[(sample & 0xf000) >> 12] >> 4; Their encoder, which works perfectly of course, is: libavcodec/s302menc.c: for (c = 0; c < frame->nb_samples; c++) { uint8_t vucf = s->framing_index == 0 ? 0x10 : 0; for (channels = 0; channels < avctx->channels; channels += 2) { o[0] = ff_reverse[ samples[0] & 0xFF]; o[1] = ff_reverse[(samples[0] & 0xFF00) >> 8]; o[2] = ff_reverse[(samples[1] & 0x0F) << 4] | vucf; o[3] = ff_reverse[(samples[1] & 0x0FF0) >> 4]; o[4] = ff_reverse[(samples[1] & 0xF000) >> 12]; o += 5; samples += 2; } s->framing_index++; if (s->framing_index >= 192) s->framing_index = 0; } In which samples[0] and samples[1] are the same for stereo audio. If you feel like we should only address this later I can drop this subject but as we've both established, the encoder in v10 is flat out wrong. Your call :) -- thanks -- Daniel
WARNING: multiple messages have this Message-ID (diff)
From: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Cc: "r.verdejo@samsung.com" <r.verdejo@samsung.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "nicolas@ndufresne.ca" <nicolas@ndufresne.ca>, "linux-kernel-mentees@lists.linuxfoundation.org" <linux-kernel-mentees@lists.linuxfoundation.org>, "linux-media@vger.kernel.org" <linux-media@vger.kernel.org> Subject: Re: [Linux-kernel-mentees] [v10 0/4] media: vidtv: Implement a virtual DVB driver Date: Sat, 12 Sep 2020 11:49:01 -0300 [thread overview] Message-ID: <15028D87-A496-4CAD-91A6-E4467489C4D0@getmailspring.com> (raw) In-Reply-To: <20200912111506.2d2bd512@coco.lan> Hi Hans and Mauro & all >Why the dvb_ prefix? All virtual drivers just start with 'vi'. > >And wouldn't it make more sense to call dvb_vidtv_bridge.ko just vidtv.ko? >Just like the other virtual media drivers? I guess Mauro was the one to come up with the dvb_* prefix for the kernel modules for the reasons he explicited up in this thread. As far as dvb_vidtv_bridge.ko and vidtv_bridge.c, I just wanted to be verbose so that people would look at this and see that it is the code for a bridge driver, since this is also supposed to be a template. Also because I had some trouble myself figuring out what was what when first browsing through other dvb drivers. That said, I am 100% onboard with renaming this to vidtv.ko or whatever seems more appropiate :) >Yet, I agree with you that at least an alias is needed. >earlier today, I wrote a patch with such purpose: If you all would like to just leave this at that ^ I am also ok with it. >For regression testing of vidtv during the daily build it would be great if >the contrib/test/test-media script can be enhanced to include vidtv. Sure, I can do that if you'd like. Can you provide some tips on how to get started? :) >Note that this script relies on the /dev/mediaX devices to run the tests. I >noticed that vidtv doesn't appear to create a /dev/mediaX device, even though >CONFIG_MEDIA_CONTROLLER_DVB=y. This is definitely something that would be good >to support in vidtv. I didn't write any code for this specifically. I was under the impression that the dvb core would create any/all the necessary files. Mauro? >I'm thinking on something like: > >echo 1 >/sys/kernel/debug/vidtv/discontinuity > >to generate a frame numbering discontinuity > >and other things like that, changing S/N ratio and other parameters >and injecting errors, in order to test the effects on user apps. Can you provide an initial list of things you would like to see? I can then implement these and we can go from there When you say 'frame numbering discontinuity', do you mean having a gap in the the TS continuity counter for a given pid? For the s302m encoder, maybe we can add some noise to the samples? > changing S/N ratio Btw 'lose lock if signal goes bad' code in vidtv_tuner.c and vidtv_demod.c does not work currently. Mainly because they expect an array of valid frequencies to compare to and as of now there's no default. I mean these, just to be clear: static unsigned int vidtv_valid_dvb_t_freqs[NUM_VALID_TUNER_FREQS]; module_param_array(vidtv_valid_dvb_t_freqs, uint, NULL, 0); MODULE_PARM_DESC(vidtv_valid_dvb_t_freqs, "Valid DVB-T frequencies to simulate"); static unsigned int vidtv_valid_dvb_c_freqs[NUM_VALID_TUNER_FREQS]; module_param_array(vidtv_valid_dvb_c_freqs, uint, NULL, 0); MODULE_PARM_DESC(vidtv_valid_dvb_c_freqs, "Valid DVB-C frequencies to simulate"); static unsigned int vidtv_valid_dvb_s_freqs[NUM_VALID_TUNER_FREQS]; module_param_array(vidtv_valid_dvb_s_freqs, uint, NULL, 0); MODULE_PARM_DESC(vidtv_valid_dvb_s_freqs, "Valid DVB-C frequencies to simulate"); static unsigned int max_frequency_shift_hz; module_param(max_frequency_shift_hz, uint, 0); MODULE_PARM_DESC(max_frequency_shift_hz, "Maximum shift in HZ allowed when tuning in a channel") > No. I meant the audio was not a sinusoidal wave after such change. This encoder is dead simple and yet I have been struggling :/ Here is the relevant decoding code in libavcodec/s302m.c: for (; buf_size > 4; buf_size -= 5) { *o++ = (ff_reverse[buf[1]] << 8) | ff_reverse[buf[0]]; *o++ = (ff_reverse[buf[4] & 0xf0] << 12) | (ff_reverse[buf[3]] << 4) | (ff_reverse[buf[2]] >> 4); buf += 5; } I am somewhat confident that the opposite to the above is precisely what I sent you last: > f.data[0] = reverse[sample & 0xff]; > f.data[1] = reverse[(sample & 0xff00) >> 8]; > f.data[2] = (vucf << 4) | (reverse[(sample & 0x0f)] >> 4); > f.data[3] = reverse[(sample & 0x0ff0) >> 4]; > f.data[4] = reverse[(sample & 0xf000) >> 12] >> 4; Their encoder, which works perfectly of course, is: libavcodec/s302menc.c: for (c = 0; c < frame->nb_samples; c++) { uint8_t vucf = s->framing_index == 0 ? 0x10 : 0; for (channels = 0; channels < avctx->channels; channels += 2) { o[0] = ff_reverse[ samples[0] & 0xFF]; o[1] = ff_reverse[(samples[0] & 0xFF00) >> 8]; o[2] = ff_reverse[(samples[1] & 0x0F) << 4] | vucf; o[3] = ff_reverse[(samples[1] & 0x0FF0) >> 4]; o[4] = ff_reverse[(samples[1] & 0xF000) >> 12]; o += 5; samples += 2; } s->framing_index++; if (s->framing_index >= 192) s->framing_index = 0; } In which samples[0] and samples[1] are the same for stereo audio. If you feel like we should only address this later I can drop this subject but as we've both established, the encoder in v10 is flat out wrong. Your call :) -- thanks -- Daniel _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2020-09-12 14:52 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-21 12:58 [v10 0/4] media: vidtv: Implement a virtual DVB driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-08-21 12:58 ` [v10 1/4] media: vidtv: implement a tuner driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-08-21 12:58 ` [v10 2/4] media: vidtv: implement a demodulator driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-08-21 12:58 ` [v10 3/4] media: vidtv: add a bridge driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-09-15 11:53 ` Geert Uytterhoeven 2020-09-15 11:53 ` [Linux-kernel-mentees] " Geert Uytterhoeven 2020-09-15 13:26 ` Daniel W. S. Almeida 2020-09-15 13:26 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-09-15 13:35 ` Geert Uytterhoeven 2020-09-15 13:35 ` [Linux-kernel-mentees] " Geert Uytterhoeven 2020-09-15 18:13 ` Daniel W. S. Almeida 2020-09-15 18:13 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-09-16 7:01 ` Mauro Carvalho Chehab 2020-09-16 7:09 ` Geert Uytterhoeven 2020-08-21 12:58 ` [v10 4/4] media: Documentation: vidtv: Add ReST documentation for vidtv Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-09-11 8:02 ` [v10 0/4] media: vidtv: Implement a virtual DVB driver Mauro Carvalho Chehab 2020-09-11 8:02 ` [Linux-kernel-mentees] " Mauro Carvalho Chehab 2020-09-11 12:18 ` Daniel W. S. Almeida 2020-09-11 12:18 ` [Linux-kernel-mentees] " Daniel W. S. Almeida 2020-09-11 13:10 ` Mauro Carvalho Chehab 2020-09-11 13:56 ` Mauro Carvalho Chehab 2020-09-12 2:54 ` Daniel W. S. Almeida 2020-09-12 7:38 ` Mauro Carvalho Chehab 2020-09-12 8:21 ` Hans Verkuil 2020-09-12 8:21 ` [Linux-kernel-mentees] " Hans Verkuil 2020-09-12 9:15 ` Mauro Carvalho Chehab 2020-09-12 9:15 ` [Linux-kernel-mentees] " Mauro Carvalho Chehab 2020-09-12 14:49 ` Daniel W. S. Almeida [this message] 2020-09-12 14:49 ` Daniel W. S. Almeida 2020-09-12 17:57 ` Mauro Carvalho Chehab 2020-09-12 17:57 ` [Linux-kernel-mentees] " Mauro Carvalho Chehab 2020-09-14 8:33 ` Hans Verkuil 2020-09-14 8:33 ` [Linux-kernel-mentees] " Hans Verkuil 2020-09-12 8:35 ` Hans Verkuil 2020-09-12 8:35 ` [Linux-kernel-mentees] " Hans Verkuil
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=15028D87-A496-4CAD-91A6-E4467489C4D0@getmailspring.com \ --to=dwlsalmeida@gmail.com \ --cc=hverkuil@xs4all.nl \ --cc=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab+huawei@kernel.org \ --cc=nicolas@ndufresne.ca \ --cc=r.verdejo@samsung.com \ --cc=skhan@linuxfoundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.