Mailing List Archive: ivtv: devel

cx18: [PATCH] make cx18_v4l2_enc_poll() return value depend on q_io as well as q_full

Index | Next | Previous | View Threaded

cwalls at radix

Mar 5, 2008, 8:29 PM

Post #1 of 7 (1957 views)
Permalink

cx18: [PATCH] make cx18_v4l2_enc_poll() return value depend on q_io as well as q_full

Attached is a patch to have cx18_v4l2_enc_poll() to return that data is
available for reading when q_io has data. The case could occur that
q_full was empty, but q_io had data waiting to be read, in which case
cx18_v4l2_enc_poll() probably should have returned data is ready for
reading.

-Andy

This a log demonstrating the situation that can occur without this
change:

Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: open_id: 32 stream type = 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream s_flags = 0x118
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream buffers 64 buf_size 32768 buffers_stolen 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_free next 0x295d0fc0 prev 295d0340 buffers 63 length 2064384 bytesused 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_full next 0x3b0a0208 prev 3b0a0208 buffers 0 length 0 bytesused 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_io next 0x295d0cc0 prev 295d0cc0 buffers 1 length 32768 bytesused 28672
Mar 5 22:45:10 palomino kernel: cx180 file: Encoder poll
Mar 5 22:45:10 palomino kernel: cx180 file: Post poll_wait
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream s_flags = 0x118
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream buffers 64 buf_size 32768 buffers_stolen 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_free next 0x295d0fc0 prev 295d0340 buffers 63 length 2064384 bytesused 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_full next 0x3b0a0208 prev 3b0a0208 buffers 0 length 0 bytesused 0
Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_io next 0x295d0cc0 prev 295d0cc0 buffers 1 length 32768 bytesused 28672
... MythTV gives up here and closes and reopens the device node,
but data could have been read if select() had properly notified MythTV.
Mar 5 22:45:10 palomino kernel: cx18-0 info: close stopping capture
Mar 5 22:45:10 palomino kernel: cx18-0 info: close stopping embedded VBI capture
Mar 5 22:45:10 palomino kernel: cx18-0 info: Stop Capture
Mar 5 22:45:11 palomino kernel: cx18-0 info: Stop Capture
Mar 5 22:45:11 palomino kernel: cx18-0 file: open encoder MPEG
Mar 5 22:45:11 palomino kernel: cx180 file: enc_poll: open_id: 33 stream type = 0

Attachments:

cx18-poll-q_io.patch (0.47 KB)

wrsturm at shaw

Mar 5, 2008, 9:35 PM

Post #2 of 7 (1886 views)
Permalink

Re: cx18: [PATCH] make cx18_v4l2_enc_poll() return value depend on q_io as well as q_full [In reply to]

On Wed, 2008-03-05 at 23:29 -0500, Andy Walls wrote:
> Attached is a patch to have cx18_v4l2_enc_poll() to return that data is
> available for reading when q_io has data. The case could occur that
> q_full was empty, but q_io had data waiting to be read, in which case
> cx18_v4l2_enc_poll() probably should have returned data is ready for
> reading.
>

Hi Andy.

I tried this patch and mythtv does indeed seem happy. The debug output
is a little different and I get some negative bytesused values. may
have something to do with the compile warnings on the debug lines
(pointer versus unsigned int).

There is an occasional audio blip.

> -Andy
>
> This a log demonstrating the situation that can occur without this
> change:
>
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: open_id: 32 stream type = 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream s_flags = 0x118
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream buffers 64 buf_size 32768 buffers_stolen 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_free next 0x295d0fc0 prev 295d0340 buffers 63 length 2064384 bytesused 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_full next 0x3b0a0208 prev 3b0a0208 buffers 0 length 0 bytesused 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_io next 0x295d0cc0 prev 295d0cc0 buffers 1 length 32768 bytesused 28672
> Mar 5 22:45:10 palomino kernel: cx180 file: Encoder poll
> Mar 5 22:45:10 palomino kernel: cx180 file: Post poll_wait
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream s_flags = 0x118
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream buffers 64 buf_size 32768 buffers_stolen 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_free next 0x295d0fc0 prev 295d0340 buffers 63 length 2064384 bytesused 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_full next 0x3b0a0208 prev 3b0a0208 buffers 0 length 0 bytesused 0
> Mar 5 22:45:10 palomino kernel: cx180 file: enc_poll: stream q_io next 0x295d0cc0 prev 295d0cc0 buffers 1 length 32768 bytesused 28672
> ... MythTV gives up here and closes and reopens the device node,
> but data could have been read if select() had properly notified MythTV.
> Mar 5 22:45:10 palomino kernel: cx18-0 info: close stopping capture
> Mar 5 22:45:10 palomino kernel: cx18-0 info: close stopping embedded VBI capture
> Mar 5 22:45:10 palomino kernel: cx18-0 info: Stop Capture
> Mar 5 22:45:11 palomino kernel: cx18-0 info: Stop Capture
> Mar 5 22:45:11 palomino kernel: cx18-0 file: open encoder MPEG
> Mar 5 22:45:11 palomino kernel: cx180 file: enc_poll: open_id: 33 stream type = 0
>
> _______________________________________________
> ivtv-devel mailing list
> ivtv-devel [at] ivtvdriver
> http://ivtvdriver.org/mailman/listinfo/ivtv-devel

_______________________________________________
ivtv-devel mailing list
ivtv-devel [at] ivtvdriver
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

hverkuil at xs4all

Mar 5, 2008, 10:55 PM

Post #3 of 7 (1884 views)
Permalink

Re: cx18: [PATCH] make cx18 v4l2 enc poll() return value depend on q io as well as q full [In reply to]

On Thursday 06 March 2008 05:29:19 Andy Walls wrote:
> Attached is a patch to have cx18_v4l2_enc_poll() to return that data
> is available for reading when q_io has data. The case could occur
> that q_full was empty, but q_io had data waiting to be read, in which
> case cx18_v4l2_enc_poll() probably should have returned data is ready
> for reading.

Nice catch! I've committed this. Interestingly enough, I think the same
bug is also present in ivtv. I'll look at that tonight.

I'll also look at the other patch tonight: I think you found a serious
bug with that one as well.

Thank you!

Hans

_______________________________________________
ivtv-devel mailing list
ivtv-devel [at] ivtvdriver
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

cwalls at radix

Mar 6, 2008, 3:35 PM

Post #4 of 7 (1868 views)
Permalink

Re: cx18: [PATCH] make cx18_v4l2_enc_poll() return value depend on q_io as well as q_full [In reply to]

Warren Sturm wrote:
> On Wed, 2008-03-05 at 23:29 -0500, Andy Walls wrote:
> > Attached is a patch to have cx18_v4l2_enc_poll() to return that data is
> > available for reading when q_io has data. The case could occur that
> > q_full was empty, but q_io had data waiting to be read, in which case
> > cx18_v4l2_enc_poll() probably should have returned data is ready for
> > reading.
>
> I tried this patch and mythtv does indeed seem happy. The debug output
> is a little different and I get some negative bytesused values. may
> have something to do with the compile warnings on the debug lines
> (pointer versus unsigned int).
>
> There is an occasional audio blip.

Warren,

Pull the latest version from:

http://www.linuxtv.org/hg/~hverkuil/cx18/archive/tip.tar.bz2

Hans has checked in this patch and the one to handle the bytesused < 0
problem. Both of them together may make the audio blips go away as
well.

-Andy

_______________________________________________
ivtv-devel mailing list
ivtv-devel [at] ivtvdriver
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

cwalls at radix

Mar 6, 2008, 4:09 PM

Post #5 of 7 (1876 views)
Permalink

Re: cx18: [PATCH] make cx18_v4l2_enc_poll() return value depend on q_io as well as q_full [In reply to]

Hans Verkuil wrote:
> On Thursday 06 March 2008 05:29:19 Andy Walls wrote:
> > Attached is a patch to have cx18_v4l2_enc_poll() to return that data
> > is available for reading when q_io has data. The case could occur
> > that q_full was empty, but q_io had data waiting to be read, in which
> > case cx18_v4l2_enc_poll() probably should have returned data is ready
> > for reading.
>
> Nice catch! I've committed this. Interestingly enough, I think the same
> bug is also present in ivtv. I'll look at that tonight.

Thanks.

Yes, I noted the bug in ivtv as well, but I left it alone since I was
tired and ivtv uses q_io differently for write streams to the cx23415's
decoder.

However this patch isn't the end of fixing poll()/select() for cx18.
Though this simple patch is correct and necessary, I suspect it will now
mask another underlying problem.

Here's my observation: There was no good reason for q_full to stay
empty for so long (> 5 seconds), just because data was sitting queued in
q_io. It was as if data stopped being moved from the encoder to buffers
and into q_full. What is both fortunate and unfortunate is that
draining q_full and q_io, seems to restart the transfers from the
encoder.

mplayer never had a problem with cx18 never providing new data
eventually, because it was always draining q_io and q_full. MythTV
would never drain q_io and the data transfers from the encoder would
never restart.

This behavior of having to drain q_full and q_io to restart the data
transfers may be why I notice blocking read() delays of about 0.1 second
happening much more often than with ivtv.

So to test these hypotheses, I'll have to gain some understanding of the
new mailbox protocol and the Memory Descriptor List stuff and do some
testing with the fix to cx18_v4l2_enc_poll() backed out.

But hey for now, the majority of HVR-1600 MythTV users should be much
happier. :)

> I'll also look at the other patch tonight: I think you found a serious
> bug with that one as well.

Thanks again. I was going for the bugs I could fix without data sheets.
That fix was a hard fought change to only 5 lines of code.

> Thank you!

You're welcome. I enjoyed working on these problems. I guess my
irrational desire for Picture-in-Picture can be a good thing at
times. ;)

-Andy

> Hans

_______________________________________________
ivtv-devel mailing list
ivtv-devel [at] ivtvdriver
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

hverkuil at xs4all

Mar 6, 2008, 11:08 PM

Post #6 of 7 (1871 views)
Permalink

Re: cx18: [PATCH] make cx18 v4l2 enc poll() return value depend on q io as well as q full [In reply to]

On Friday 07 March 2008 01:09:27 Andy Walls wrote:
> Hans Verkuil wrote:
> > On Thursday 06 March 2008 05:29:19 Andy Walls wrote:
> > > Attached is a patch to have cx18_v4l2_enc_poll() to return that
> > > data is available for reading when q_io has data. The case could
> > > occur that q_full was empty, but q_io had data waiting to be
> > > read, in which case cx18_v4l2_enc_poll() probably should have
> > > returned data is ready for reading.
> >
> > Nice catch! I've committed this. Interestingly enough, I think the
> > same bug is also present in ivtv. I'll look at that tonight.
>
> Thanks.
>
> Yes, I noted the bug in ivtv as well, but I left it alone since I was
> tired and ivtv uses q_io differently for write streams to the
> cx23415's decoder.
>
>
> However this patch isn't the end of fixing poll()/select() for cx18.
> Though this simple patch is correct and necessary, I suspect it will
> now mask another underlying problem.
>
> Here's my observation: There was no good reason for q_full to stay
> empty for so long (> 5 seconds), just because data was sitting queued
> in q_io. It was as if data stopped being moved from the encoder to
> buffers and into q_full. What is both fortunate and unfortunate is
> that draining q_full and q_io, seems to restart the transfers from
> the encoder.
>
> mplayer never had a problem with cx18 never providing new data
> eventually, because it was always draining q_io and q_full. MythTV
> would never drain q_io and the data transfers from the encoder would
> never restart.
>
> This behavior of having to drain q_full and q_io to restart the data
> transfers may be why I notice blocking read() delays of about 0.1
> second happening much more often than with ivtv.
>
> So to test these hypotheses, I'll have to gain some understanding of
> the new mailbox protocol and the Memory Descriptor List stuff and do
> some testing with the fix to cx18_v4l2_enc_poll() backed out.

Look for 'CX18_CPU_DE_SET_MDL' in cx18-fileops.c: this is where the
processed buffer is returned to the firmware so that it can be used
again. Based on your description I was wondering whether in some cases
all or most buffers are in use and the firmware will just stop
capturing due to a lack of available buffers.

> But hey for now, the majority of HVR-1600 MythTV users should be much
> happier. :)
>
> > I'll also look at the other patch tonight: I think you found a
> > serious bug with that one as well.
>
> Thanks again. I was going for the bugs I could fix without data
> sheets. That fix was a hard fought change to only 5 lines of code.
>
> > Thank you!
>
> You're welcome. I enjoyed working on these problems. I guess my
> irrational desire for Picture-in-Picture can be a good thing at
> times. ;)

Dangerous. I started out working on ivtv due to an irrational desire to
capture a widescreen signal and I ended up as ivtv maintainer :-)

Regards,

Hans

_______________________________________________
ivtv-devel mailing list
ivtv-devel [at] ivtvdriver
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

santa8claws at yahoo

Mar 7, 2008, 7:12 AM

Post #7 of 7 (1866 views)
Permalink

Re: cx18: [PATCH] make cx18_v4l2_enc_poll() return value [In reply to]

This is fantastic! With the latest patches my hvr-1600 works great in my MythDora setup (Fedora Core 6). Good work finding this bug and tracking it down. I hope to see alot more people get involved and start to use this card in linux.

I have found though that ivtv-tune doesn't like to be run from MythTV backend, it causes a Segmentation Fault although running from bash works fine. Anyone know how to get channel changing working in MythTV?

--
This message was sent on behalf of santa8claws [at] yahoo at openSubscriber.com
http://www.opensubscriber.com/message/ivtv-devel [at] ivtvdriver/8747085.html

_______________________________________________
ivtv-devel mailing list
ivtv-devel [at] ivtvdriver
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads