Refactor videotest driver #69
Conversation
| @@ -47,12 +47,10 @@ func (d *dummy) Close() error { | |||
| } | |||
|
|
|||
| func (d *dummy) VideoRecord(p prop.Media) (video.Reader, error) { | |||
| yi := p.Width * p.Height | |||
| ci := yi / 2 | |||
| if p.FrameRate == 0 { | |||
There was a problem hiding this comment.
This seems to be a very common thing, having default values for some properties. I feel like we should automatically do it in the wrapper instead. What do you think?
There was a problem hiding this comment.
I think default value may depend on the device.
It might be better to have an interface to get default prop by the wrapper.
btw, #61 is bit related.
videotest and screen can take any framerate, but the value the user specified is not passed to VideoRecord at now.
There was a problem hiding this comment.
Ah right. I think that I missed the main problem. I think the problem is not actually about having the default value or not. But, the problem is how we handle non-discrete values, for example frame rate.
There was a problem hiding this comment.
I'll continue the conversation in #61. I think the design decision for this can take a while.
pkg/driver/videotest/dummy.go
Outdated
| if y > hColorBarEnd { | ||
| } else { |
There was a problem hiding this comment.
I think this one should be just if y <= hColorBarEnd {
There was a problem hiding this comment.
oh, this if is no longer needed as I splitted for loop as for y := 0; y < hColorBarEnd; y++ {.
pkg/driver/videotest/dummy.go
Outdated
| cbBase[ci+x/2] = colors[c][1] | ||
| crBase[ci+x/2] = colors[c][2] |
There was a problem hiding this comment.
I like it. It definitely looks better than having an if statement for just handling Chroma subsampling
Cache fixed region to reduce CPU usage.
at-wat
force-pushed
the
refactor-videotest
branch
from
7a42557
to
1e87b71
Compare
It took too much CPU time.
Cache fixed region to reduce CPU usage.