| Message ID | CAGTf1Mkj91-8jGeRXmWwVW5Pusk6vQLwcpMgaygCEQqj8YbefQ@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Gents, Could somebody please take a look into my patch? Or is it somehow invisible / badly formatted? It allows for inducing key frames at proper moments by e.g. -force_key_frames, while using openH264 codec. Thus accurate HLS with LGPL license, which is important for us. Valery On Monday, March 5, 2018, Valery Kot <valery.kot@gmail.com> wrote: > libx264 encoder uses AVFrame.pict_type (if provided by user) to use this > frame as key frame. > > For openh264 encoder this functionality is missing, pict_type simply being > ignored. > > Attached patch fixes it. > > Valery >
On 3/12/2018 6:58 AM, Valery Kot wrote: > Could somebody please take a look into my patch? Or is it somehow invisible > / badly formatted? > > It allows for inducing key frames at proper moments by e.g. > -force_key_frames, while using openH264 codec. Thus accurate HLS with LGPL > license, which is important for us. Hi, > + if (frame->pict_type==AV_PICTURE_TYPE_I) { > + (*s->encoder)->ForceIntraFrame(s->encoder, true); > + } Does openh264 differentiate between I and IDR frames in its API, like libx264 and libx265 do? - Derek
On Mon, 5 Mar 2018 15:01:16 +0100 Valery Kot <valery.kot@gmail.com> wrote: > From f95943165c91dac13a644365f775aff3dd9edb11 Mon Sep 17 00:00:00 2001 > From: vkot <valery.kot@4cinsights.com> > Date: Mon, 5 Mar 2018 13:51:51 +0100 > Subject: [PATCH 3/3] avcodec/openh264enc.c: generate IDR frame in response to > I frame pict_type > > --- > libavcodec/libopenh264enc.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > index fdadb101f5..12e9ad49ed 100644 > --- a/libavcodec/libopenh264enc.c > +++ b/libavcodec/libopenh264enc.c > @@ -245,6 +245,10 @@ static int svc_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > } > sp.iPicWidth = avctx->width; > sp.iPicHeight = avctx->height; > + The above line has trailing whitespace which should be avoided. You can use tools/patcheck to check, and git should provide a warning when applying the patch.
Got it. Do I have to post an updated patch as a reply to this thread? Valery
On Mon, Mar 12, 2018, at 1:46 PM, Valery Kot wrote:
> Got it. Do I have to post an updated patch as a reply to this thread?
Whatever you prefer, but adding a version to the subject can be helpful for us to keep track. You can do that with the "-v" option in "git format-patch". If you want to keep it within the thread use the "--in-reply-to" option (view the raw email message or message headers to get the in-reply-to value).
But you don't necessarily need to make a new patch to address the minor whitespace issue. You can wait for other comments and include it with any other requested changes.
Please try to avoid top-posting on the mailing list: it makes it harder to follow the replies. Lastly, if you can fix the reply quoting in your email message that would be helpful as well.
On Mon, Mar 12, 2018 at 14:38:21 -0800, Lou Logan wrote: > But you don't necessarily need to make a new patch to address the > minor whitespace issue. You can wait for other comments and include > it with any other requested changes. Another whitespace nit: > if (frame->pict_type==AV_PICTURE_TYPE_I) { Please use single spaces around operators, i.e. if (frame->pict_type == AV_PICTURE_TYPE_I) { Moritz
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c index fdadb101f5..12e9ad49ed 100644 --- a/libavcodec/libopenh264enc.c +++ b/libavcodec/libopenh264enc.c @@ -245,6 +245,10 @@ static int svc_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, } sp.iPicWidth = avctx->width; sp.iPicHeight = avctx->height; + + if (frame->pict_type==AV_PICTURE_TYPE_I) { + (*s->encoder)->ForceIntraFrame(s->encoder, true); + } encoded = (*s->encoder)->EncodeFrame(s->encoder, &sp, &fbi); if (encoded != cmResultSuccess) {
libx264 encoder uses AVFrame.pict_type (if provided by user) to use this frame as key frame. For openh264 encoder this functionality is missing, pict_type simply being ignored. Attached patch fixes it. Valery From f95943165c91dac13a644365f775aff3dd9edb11 Mon Sep 17 00:00:00 2001 From: vkot <valery.kot@4cinsights.com> Date: Mon, 5 Mar 2018 13:51:51 +0100 Subject: [PATCH 3/3] avcodec/openh264enc.c: generate IDR frame in response to I frame pict_type --- libavcodec/libopenh264enc.c | 4 ++++ 1 file changed, 4 insertions(+)