From: Daniel W. S. Almeida <[email protected]>
Fix the following error for builds on 32bit architectures:
ERROR: modpost: "__udivdi3"
[drivers/media/test-drivers/vidtv/dvb-vidtv-bridge.ko] undefined!
Which is due to 64bit divisions that did not go through the helpers
in linux/math64.h
As vidtv_mux_check_mux_rate was not operational in its current form,
drop the entire function while it is not fixed properly.
For now, call vidtv_mux_pad_with_nulls with a constant number of packets
to avoid warnings due to unused functions when building this driver.
Fixes: f90cf6079bf67988 ("media: vidtv: add a bridge driver")
Signed-off-by: Daniel W. S. Almeida <[email protected]>
---
drivers/media/test-drivers/vidtv/vidtv_mux.c | 34 +------------------
.../media/test-drivers/vidtv/vidtv_s302m.c | 4 +--
2 files changed, 3 insertions(+), 35 deletions(-)
diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
index 5d1a275d504b..6e402a880fdc 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
@@ -336,38 +336,6 @@ static u32 vidtv_mux_pad_with_nulls(struct vidtv_mux *m, u32 npkts)
return nbytes;
}
-static u32 vidtv_mux_check_mux_rate(struct vidtv_mux *m)
-{
- /*
- * attempt to maintain a constant mux rate, padding with null packets
- * if needed
- */
-
- u32 nbytes = 0; /* the number of bytes written by this function */
-
- u64 nbytes_expected; /* the number of bytes we should have written */
- u64 nbytes_streamed; /* the number of bytes we actually wrote */
- u32 num_null_pkts; /* number of null packets to bridge the gap */
-
- u64 elapsed_time_msecs = jiffies_to_usecs(m->timing.current_jiffies -
- m->timing.past_jiffies);
-
- elapsed_time_msecs = min(elapsed_time_msecs, (u64)VIDTV_MAX_SLEEP_USECS / 1000);
- nbytes_expected = div64_u64(m->mux_rate_kbytes_sec * 1000, MSEC_PER_SEC);
- nbytes_expected *= elapsed_time_msecs;
-
- nbytes_streamed = m->mux_buf_offset;
-
- if (nbytes_streamed < nbytes_expected) {
- /* can't write half a packet: roundup to a 188 multiple */
- nbytes_expected = roundup(nbytes_expected - nbytes_streamed, TS_PACKET_LEN);
- num_null_pkts = nbytes_expected / TS_PACKET_LEN;
- nbytes += vidtv_mux_pad_with_nulls(m, num_null_pkts);
- }
-
- return nbytes;
-}
-
static void vidtv_mux_clear(struct vidtv_mux *m)
{
/* clear the packets currently in the mux */
@@ -397,7 +365,7 @@ static void vidtv_mux_tick(struct work_struct *work)
nbytes += vidtv_mux_push_si(m);
nbytes += vidtv_mux_poll_encoders(m);
- nbytes += vidtv_mux_check_mux_rate(m);
+ nbytes += vidtv_mux_pad_with_nulls(m, 256);
npkts = nbytes / TS_PACKET_LEN;
diff --git a/drivers/media/test-drivers/vidtv/vidtv_s302m.c b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
index f8049cdf564a..e3290facf57b 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
@@ -285,12 +285,12 @@ static void vidtv_s302m_compute_pts(struct vidtv_encoder *e)
{
u64 count = e->sample_count;
struct vidtv_access_unit *au = e->access_units;
+ u32 duration = CLOCK_UNIT_90KHZ / e->sampling_rate_hz;
while (au) {
count += au->num_samples;
- au->pts = count *
- CLOCK_UNIT_90KHZ / e->sampling_rate_hz;
+ au->pts = count * duration;
au = au->next;
}
--
2.28.0
On 9/15/20 11:05 AM, Daniel W. S. Almeida wrote:
> From: Daniel W. S. Almeida <[email protected]>
>
> Fix the following error for builds on 32bit architectures:
>
> ERROR: modpost: "__udivdi3"
> [drivers/media/test-drivers/vidtv/dvb-vidtv-bridge.ko] undefined!
>
> Which is due to 64bit divisions that did not go through the helpers
> in linux/math64.h
>
> As vidtv_mux_check_mux_rate was not operational in its current form,
> drop the entire function while it is not fixed properly.
>
> For now, call vidtv_mux_pad_with_nulls with a constant number of packets
> to avoid warnings due to unused functions when building this driver.
>
> Fixes: f90cf6079bf67988 ("media: vidtv: add a bridge driver")
> Signed-off-by: Daniel W. S. Almeida <[email protected]>
> ---
> drivers/media/test-drivers/vidtv/vidtv_mux.c | 34 +------------------
> .../media/test-drivers/vidtv/vidtv_s302m.c | 4 +--
> 2 files changed, 3 insertions(+), 35 deletions(-)
>
Works for me. Thanks.
Acked-by: Randy Dunlap <[email protected]> # build-tested
--
~Randy
Em Tue, 15 Sep 2020 15:05:09 -0300
"Daniel W. S. Almeida" <[email protected]> escreveu:
> From: Daniel W. S. Almeida <[email protected]>
>
> Fix the following error for builds on 32bit architectures:
>
> ERROR: modpost: "__udivdi3"
> [drivers/media/test-drivers/vidtv/dvb-vidtv-bridge.ko] undefined!
>
> Which is due to 64bit divisions that did not go through the helpers
> in linux/math64.h
>
> As vidtv_mux_check_mux_rate was not operational in its current form,
> drop the entire function while it is not fixed properly.
>
> For now, call vidtv_mux_pad_with_nulls with a constant number of packets
> to avoid warnings due to unused functions when building this driver.
>
> Fixes: f90cf6079bf67988 ("media: vidtv: add a bridge driver")
> Signed-off-by: Daniel W. S. Almeida <[email protected]>
> ---
> drivers/media/test-drivers/vidtv/vidtv_mux.c | 34 +------------------
> .../media/test-drivers/vidtv/vidtv_s302m.c | 4 +--
> 2 files changed, 3 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/media/test-drivers/vidtv/vidtv_mux.c b/drivers/media/test-drivers/vidtv/vidtv_mux.c
> index 5d1a275d504b..6e402a880fdc 100644
> --- a/drivers/media/test-drivers/vidtv/vidtv_mux.c
> +++ b/drivers/media/test-drivers/vidtv/vidtv_mux.c
> @@ -336,38 +336,6 @@ static u32 vidtv_mux_pad_with_nulls(struct vidtv_mux *m, u32 npkts)
> return nbytes;
> }
>
> -static u32 vidtv_mux_check_mux_rate(struct vidtv_mux *m)
> -{
> - /*
> - * attempt to maintain a constant mux rate, padding with null packets
> - * if needed
> - */
> -
> - u32 nbytes = 0; /* the number of bytes written by this function */
> -
> - u64 nbytes_expected; /* the number of bytes we should have written */
> - u64 nbytes_streamed; /* the number of bytes we actually wrote */
> - u32 num_null_pkts; /* number of null packets to bridge the gap */
> -
> - u64 elapsed_time_msecs = jiffies_to_usecs(m->timing.current_jiffies -
> - m->timing.past_jiffies);
> -
> - elapsed_time_msecs = min(elapsed_time_msecs, (u64)VIDTV_MAX_SLEEP_USECS / 1000);
> - nbytes_expected = div64_u64(m->mux_rate_kbytes_sec * 1000, MSEC_PER_SEC);
> - nbytes_expected *= elapsed_time_msecs;
> -
> - nbytes_streamed = m->mux_buf_offset;
> -
> - if (nbytes_streamed < nbytes_expected) {
> - /* can't write half a packet: roundup to a 188 multiple */
> - nbytes_expected = roundup(nbytes_expected - nbytes_streamed, TS_PACKET_LEN);
> - num_null_pkts = nbytes_expected / TS_PACKET_LEN;
> - nbytes += vidtv_mux_pad_with_nulls(m, num_null_pkts);
> - }
> -
> - return nbytes;
> -}
> -
> static void vidtv_mux_clear(struct vidtv_mux *m)
> {
> /* clear the packets currently in the mux */
> @@ -397,7 +365,7 @@ static void vidtv_mux_tick(struct work_struct *work)
> nbytes += vidtv_mux_push_si(m);
>
> nbytes += vidtv_mux_poll_encoders(m);
> - nbytes += vidtv_mux_check_mux_rate(m);
> + nbytes += vidtv_mux_pad_with_nulls(m, 256);
>
> npkts = nbytes / TS_PACKET_LEN;
>
> diff --git a/drivers/media/test-drivers/vidtv/vidtv_s302m.c b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> index f8049cdf564a..e3290facf57b 100644
> --- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> +++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> @@ -285,12 +285,12 @@ static void vidtv_s302m_compute_pts(struct vidtv_encoder *e)
> {
> u64 count = e->sample_count;
> struct vidtv_access_unit *au = e->access_units;
> + u32 duration = CLOCK_UNIT_90KHZ / e->sampling_rate_hz;
>
> while (au) {
> count += au->num_samples;
>
> - au->pts = count *
> - CLOCK_UNIT_90KHZ / e->sampling_rate_hz;
> + au->pts = count * duration;
That doesn't seem to be the right thing to do here.
Assuming that sampling rate is 48 kHz, you'll
have duration = 1.875, which would be rounded to 1.
In other words, the above is identical to:
au->pts = count
Now, I don't know from where that CLOCK_UNIT_90KHZ came from.
If such constant is not needed anymore, just drop it.
If, on the other hand, this is required by the specs, then
you may need to do a 64 bits division, e. g. using
div64_u64() or do_div().
Thanks,
Mauro
Hi Mauro, Daniel,
On Wed, Sep 16, 2020 at 8:40 AM Mauro Carvalho Chehab
<[email protected]> wrote:
> Em Tue, 15 Sep 2020 15:05:09 -0300
> "Daniel W. S. Almeida" <[email protected]> escreveu:
> > From: Daniel W. S. Almeida <[email protected]>
> >
> > Fix the following error for builds on 32bit architectures:
> >
> > ERROR: modpost: "__udivdi3"
> > [drivers/media/test-drivers/vidtv/dvb-vidtv-bridge.ko] undefined!
> >
> > Which is due to 64bit divisions that did not go through the helpers
> > in linux/math64.h
> >
> > As vidtv_mux_check_mux_rate was not operational in its current form,
> > drop the entire function while it is not fixed properly.
> >
> > For now, call vidtv_mux_pad_with_nulls with a constant number of packets
> > to avoid warnings due to unused functions when building this driver.
> >
> > Fixes: f90cf6079bf67988 ("media: vidtv: add a bridge driver")
> > Signed-off-by: Daniel W. S. Almeida <[email protected]>
Acked-by: Geert Uytterhoeven <[email protected]> # build-tested
> > --- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> > +++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> > @@ -285,12 +285,12 @@ static void vidtv_s302m_compute_pts(struct vidtv_encoder *e)
> > {
> > u64 count = e->sample_count;
> > struct vidtv_access_unit *au = e->access_units;
> > + u32 duration = CLOCK_UNIT_90KHZ / e->sampling_rate_hz;
> >
> > while (au) {
> > count += au->num_samples;
> >
> > - au->pts = count *
> > - CLOCK_UNIT_90KHZ / e->sampling_rate_hz;
> > + au->pts = count * duration;
>
> That doesn't seem to be the right thing to do here.
>
> Assuming that sampling rate is 48 kHz, you'll
> have duration = 1.875, which would be rounded to 1.
>
> In other words, the above is identical to:
>
> au->pts = count
>
> Now, I don't know from where that CLOCK_UNIT_90KHZ came from.
>
> If such constant is not needed anymore, just drop it.
>
> If, on the other hand, this is required by the specs, then
> you may need to do a 64 bits division, e. g. using
> div64_u64() or do_div().
As vidtv_encoder.sampling_rate_hz is u32, there's fortunately no need
to use div64_u64() (64-by-64), so div_u64() (64-by-32) is fine.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Mauro, Geert,
> That doesn't seem to be the right thing to do here.
>
> Assuming that sampling rate is 48 kHz, you'll
> have duration = 1.875, which would be rounded to 1.
>
> In other words, the above is identical to:
>
> au->pts = count
>
> Now, I don't know from where that CLOCK_UNIT_90KHZ came from.
>
Mauro, here's my current understanding:
The values for PTS have to be defined in terms of a 90khz clock.
So for 30fps video, the values for PTS per frame would be:
frame_num/30 * 90000
so 0, 3000, 6000, 9000...etc
The same math is being used here for audio, but instead we have the
total audio sample count and a 48000 samples/sec
I did miss this rounding error though, I'm sorry.