On 09/14/2015 07:48 AM, Junghak Sung wrote:
>
>
> On 09/11/2015 05:02 PM, Hans Verkuil wrote:
>> On 09/09/2015 01:19 PM, Junghak Sung wrote:
>>> Make videobuf2-v4l2 as a wrapper of videobuf2-core for v4l2-use.
>>> And replace videobuf2-core.h with videobuf2-v4l2.h.
>>> This renaming change should be accompanied by the modifications
>>> of all device drivers that include videobuf2-core.h.
>>> It can be done with just running this shell script.
>>>
>>> replace()
>>> {
>>> str1=$1
>>> str2=$2
>>> dir=$3
>>> for file in $(find $dir -name *.h -o -name *.c -o -name Makefile)
>>> do
>>> echo $file
>>> sed "s/$str1/$str2/g" $file > $file.out
>>> mv $file.out $file
>>> done
>>> }
>>>
>>> replace "videobuf2-core" "videobuf2-v4l2" "include/media/"
>>> replace "videobuf2-core" "videobuf2-v4l2" "drivers/media/"
>>> replace "videobuf2-core" "videobuf2-v4l2" "drivers/usb/gadget/"
>>>
>>> Signed-off-by: Junghak Sung <jh1009.sung@xxxxxxxxxxx>
>>> Signed-off-by: Geunyoung Kim <nenggun.kim@xxxxxxxxxxx>
>>> Acked-by: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
>>> Acked-by: Inki Dae <inki.dae@xxxxxxxxxxx>
>>
>> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>
>> However, see one small comment below:
>>
>> <snip>
>>
>>> diff --git a/include/media/videobuf2-dvb.h b/include/media/videobuf2-dvb.h
>>> index 8f61456..bef9127 100644
>>> --- a/include/media/videobuf2-dvb.h
>>> +++ b/include/media/videobuf2-dvb.h
>>> @@ -6,7 +6,7 @@
>>> #include <dvb_demux.h>
>>> #include <dvb_net.h>
>>> #include <dvb_frontend.h>
>>> -#include <media/videobuf2-core.h>
>>> +#include <media/videobuf2-v4l2.h>
>>
>> I actually think this should remain core.h since videobuf2-dvb.c/h has
>> nothing to do with v4l2.
>>
>> Regards,
>>
>> Hans
>>
>
> I agree with you.
> In this patch series, vb2_thread_*() are still remained in
> videobuf2-v4l2, because vb2_tread_*() and vb2_fileio_*() have
> dependencies on v4l2 and it is not easy to remove them completely.
Ah, OK. It's for the thread usage.
> I'll try to remove the dependencies and make
> videobuf2-dvb to include videobuf2-core instead of videobuf2-v4l2
> at next round.
I would postpone this. Instead, in the dvb.h header you should add a
comment that explains why you need v4l2.h.
Later we should look into moving the file/threadio code to core. It
really belongs there, but it might be a bit tricky to split off the
v4l2-specific bits. Of course, if you think you can keep the file/threadio
code in core.c, then feel free to do so. It would be nice.
That reminds me: you moved some of the 'if (vb2_fileio_is_active(q))'
to v4l2.c, but perhaps those should remain in core.c. We really want to
have file/threadio support in the core eventually, so keeping those
checks in core.c avoids unnecessary code movements.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
