| Message ID | 1377829038-4726-15-git-send-email-posciak@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/30/2013 04:17 AM, Pawel Osciak wrote: > Add bits for previous, golden and altref frame types. > > Signed-off-by: Pawel Osciak <posciak@chromium.org> Kamil, is this something that applies as well to your MFC driver? > --- > include/uapi/linux/videodev2.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 437f1b0..c011ee0 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -687,6 +687,10 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN 0x0000 > #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC 0x2000 > #define V4L2_BUF_FLAG_TIMESTAMP_COPY 0x4000 > +/* VP8 special frames */ > +#define V4L2_BUF_FLAG_PREV_FRAME 0x10000 /* VP8 prev frame */ > +#define V4L2_BUF_FLAG_GOLDEN_FRAME 0x20000 /* VP8 golden frame */ > +#define V4L2_BUF_FLAG_ALTREF_FRAME 0x40000 /* VP8 altref frame */ Would it be an idea to use the same bit values as for KEYFRAME/PFRAME/BFRAME? After all, these can never be used at the same time. I'm a bit worried that the bits in this field are eventually all used up by different encoder flags. Regards, Hans > > /** > * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri 30 August 2013 09:28:36 Pawel Osciak wrote: > On Fri, Aug 30, 2013 at 3:42 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > > On 08/30/2013 04:17 AM, Pawel Osciak wrote: > > > Add bits for previous, golden and altref frame types. > > > > > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > > > > Kamil, is this something that applies as well to your MFC driver? > > > > > --- > > > include/uapi/linux/videodev2.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/include/uapi/linux/videodev2.h > > b/include/uapi/linux/videodev2.h > > > index 437f1b0..c011ee0 100644 > > > --- a/include/uapi/linux/videodev2.h > > > +++ b/include/uapi/linux/videodev2.h > > > @@ -687,6 +687,10 @@ struct v4l2_buffer { > > > #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN 0x0000 > > > #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC 0x2000 > > > #define V4L2_BUF_FLAG_TIMESTAMP_COPY 0x4000 > > > +/* VP8 special frames */ > > > +#define V4L2_BUF_FLAG_PREV_FRAME 0x10000 /* VP8 prev frame > > */ > > > +#define V4L2_BUF_FLAG_GOLDEN_FRAME 0x20000 /* VP8 golden > > frame */ > > > +#define V4L2_BUF_FLAG_ALTREF_FRAME 0x40000 /* VP8 altref > > frame */ > > > > Would it be an idea to use the same bit values as for > > KEYFRAME/PFRAME/BFRAME? > > After all, these can never be used at the same time. I'm a bit worried > > that the > > bits in this field are eventually all used up by different encoder flags. > > > > VP8 also has a concept of I and P frames and they are orthogonal to the > concept of Prev, Golden and Alt. There is no relationship at all, i.e. we > can't infer I P from Prev Golden Alt and vice versa. Ah, OK. Me no VP8 expert :-) > For the I, P > dimension, we need one bit, but same could be said about H264, we need 2 > bits, not 3 there (if it's not I or P, it's B), and we have 3 bits > nevertheless. > For Prev Golden Alt dimension, we do need 3 bits. > Now, technically, in VP8, the first bit of the frame header indicates if > the frame is I or P, so we could technically use that in userspace and > overload I P B to mean Prev Golden Alt for VP8. But while I understand the > problem with running out of bits very well, as I and P exist in VP8 as > well, it would be pretty confusing, so it's a trade-off that should be > carefully weighted. Are prev/golden/altref frames mutually exclusive? If so, then perhaps we should use a two-bit mask instead of three bits. And those two-bits can later be expanded to more to support codecs that have more than four different frame types. Regards, Hans > > Regards, > Pawel > > > > Regards, > > > > Hans > > > > > > > > /** > > > * struct v4l2_exportbuffer - export of video buffer as DMABUF file > > descriptor > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans and Pawel,
On Fri, Aug 30, 2013 at 10:12:45AM +0200, Hans Verkuil wrote:
> Are prev/golden/altref frames mutually exclusive? If so, then perhaps we
Does that apply to other types of frames as well (key, p and b)? If yes, the
existing frame bits could be used for VP8 frame flags while the existing
codecs could keep using the old definitions, i.e. same bits, but different
macros.
Just my five euro cents.
Hi Pawel, On Fri, Aug 30, 2013 at 11:17:13AM +0900, Pawel Osciak wrote: > Add bits for previous, golden and altref frame types. > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > --- > include/uapi/linux/videodev2.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 437f1b0..c011ee0 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -687,6 +687,10 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN 0x0000 > #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC 0x2000 > #define V4L2_BUF_FLAG_TIMESTAMP_COPY 0x4000 > +/* VP8 special frames */ > +#define V4L2_BUF_FLAG_PREV_FRAME 0x10000 /* VP8 prev frame */ > +#define V4L2_BUF_FLAG_GOLDEN_FRAME 0x20000 /* VP8 golden frame */ > +#define V4L2_BUF_FLAG_ALTREF_FRAME 0x40000 /* VP8 altref frame */ Whichever way this patch is changed, could you rebased it on "v4l: Use full 32 bits for buffer flags"? It changes the definitions of the flags use 32-bit values.
Hi Pawel, Thank you for the patch. On Friday 30 August 2013 11:17:13 Pawel Osciak wrote: > Add bits for previous, golden and altref frame types. > > Signed-off-by: Pawel Osciak <posciak@chromium.org> > --- > include/uapi/linux/videodev2.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 437f1b0..c011ee0 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -687,6 +687,10 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN 0x0000 > #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC 0x2000 > #define V4L2_BUF_FLAG_TIMESTAMP_COPY 0x4000 > +/* VP8 special frames */ > +#define V4L2_BUF_FLAG_PREV_FRAME 0x10000 /* VP8 prev frame */ > +#define V4L2_BUF_FLAG_GOLDEN_FRAME 0x20000 /* VP8 golden frame */ > +#define V4L2_BUF_FLAG_ALTREF_FRAME 0x40000 /* VP8 altref frame */ This required documentation in Documentation/DocBook/media/ :-) > /** > * struct v4l2_exportbuffer - export of video buffer as DMABUF file > descriptor
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 437f1b0..c011ee0 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -687,6 +687,10 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN 0x0000 #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC 0x2000 #define V4L2_BUF_FLAG_TIMESTAMP_COPY 0x4000 +/* VP8 special frames */ +#define V4L2_BUF_FLAG_PREV_FRAME 0x10000 /* VP8 prev frame */ +#define V4L2_BUF_FLAG_GOLDEN_FRAME 0x20000 /* VP8 golden frame */ +#define V4L2_BUF_FLAG_ALTREF_FRAME 0x40000 /* VP8 altref frame */ /** * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
Add bits for previous, golden and altref frame types. Signed-off-by: Pawel Osciak <posciak@chromium.org> --- include/uapi/linux/videodev2.h | 4 ++++ 1 file changed, 4 insertions(+)