Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Michael Niedermayer
On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/proresdec2.c | 51 ++---
>  1 file changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> index 8581d797fb..80a76bbba1 100644
> --- a/libavcodec/proresdec2.c
> +++ b/libavcodec/proresdec2.c
> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext *avctx)
>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
> 
> avctx->bits_per_raw_sample = 10;
> 
>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
> switch (avctx->codec_tag) {
> case MKTAG('a','p','c','o'):
> avctx->profile = FF_PROFILE_PRORES_PROXY;
>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext *avctx)
> break;
> case MKTAG('a','p','4','h'):
> avctx->profile = FF_PROFILE_PRORES_;
>-avctx->bits_per_raw_sample = 12;
> break;
> case MKTAG('a','p','4','x'):
> avctx->profile = FF_PROFILE_PRORES_XQ;
>-avctx->bits_per_raw_sample = 12;
> break;
> default:
>-avctx->profile = FF_PROFILE_UNKNOWN;
> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile %d\n", 
> avctx->codec_tag);
> }
>+}
>+
>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
>+avctx->profile == FF_PROFILE_PRORES_)
>+avctx->bits_per_raw_sample = 12;

with this it would be possible to have 12bit output while the profile
is set to one having 10bits and vice versa ?

maybe the profile should only be left if it is compatible with the
decoder output

 
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] lavc/qsvenc: set pict_type to be I for IDR frames.

2018-12-07 Thread Zhong Li
Signed-off-by: Zhong Li 
---
 libavcodec/qsvenc.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index aa7f347..8289a32 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -1378,10 +1378,11 @@ int ff_qsv_encode(AVCodecContext *avctx, QSVEncContext 
*q,
 new_pkt.size = bs->DataLength;
 
 if (bs->FrameType & MFX_FRAMETYPE_IDR ||
-bs->FrameType & MFX_FRAMETYPE_xIDR)
+bs->FrameType & MFX_FRAMETYPE_xIDR) {
 new_pkt.flags |= AV_PKT_FLAG_KEY;
-
-if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType & 
MFX_FRAMETYPE_xI)
+pict_type = AV_PICTURE_TYPE_I;
+}
+else if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType & 
MFX_FRAMETYPE_xI)
 pict_type = AV_PICTURE_TYPE_I;
 else if (bs->FrameType & MFX_FRAMETYPE_P || bs->FrameType & 
MFX_FRAMETYPE_xP)
 pict_type = AV_PICTURE_TYPE_P;
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Paul B Mahol
On 12/7/18, Michael Niedermayer  wrote:
> On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavcodec/proresdec2.c | 51 ++---
>>  1 file changed, 27 insertions(+), 24 deletions(-)
>>
>> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
>> index 8581d797fb..80a76bbba1 100644
>> --- a/libavcodec/proresdec2.c
>> +++ b/libavcodec/proresdec2.c
>> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
>> *avctx)
>>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>
>> avctx->bits_per_raw_sample = 10;
>>
>>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
>> switch (avctx->codec_tag) {
>> case MKTAG('a','p','c','o'):
>> avctx->profile = FF_PROFILE_PRORES_PROXY;
>>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
>> *avctx)
>> break;
>> case MKTAG('a','p','4','h'):
>> avctx->profile = FF_PROFILE_PRORES_;
>>-avctx->bits_per_raw_sample = 12;
>> break;
>> case MKTAG('a','p','4','x'):
>> avctx->profile = FF_PROFILE_PRORES_XQ;
>>-avctx->bits_per_raw_sample = 12;
>> break;
>> default:
>>-avctx->profile = FF_PROFILE_UNKNOWN;
>> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile %d\n",
>> avctx->codec_tag);
>> }
>>+}
>>+
>>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
>>+avctx->profile == FF_PROFILE_PRORES_)
>>+avctx->bits_per_raw_sample = 12;
>
> with this it would be possible to have 12bit output while the profile
> is set to one having 10bits and vice versa ?

No, and neither with previous code.

> maybe the profile should only be left if it is compatible with the
> decoder output

I do not follow.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Paul B Mahol
On 12/7/18, Paul B Mahol  wrote:
> On 12/7/18, Michael Niedermayer  wrote:
>> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>>> This recovers state with #7374 linked sample.
>>>
>>> Work funded by Open Broadcast Systems.
>>>
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavcodec/h264_refs.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>>> index eaf965e43d..5645a203a7 100644
>>> --- a/libavcodec/h264_refs.c
>>> +++ b/libavcodec/h264_refs.c
>>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>>  }
>>>  break;
>>>  case MMCO_RESET:
>>> +default:
>>>  while (h->short_ref_count) {
>>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>>  }
>>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>>  h->last_pocs[j] = INT_MIN;
>>>  break;
>>> -default: assert(0);
>>>  }
>>>  }
>>
>> mmco[i].opcode should not be invalid, its checked around the point where
>> this
>> array is filled.
>> unless there is something iam missing
>
> Yes, you are missing big time.
> If you think by "checked" about those nice asserts they are not enabled at
> all.
>

There is check for invalid opcode, but stored invalid opcode is not changed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Paul B Mahol
On 12/7/18, Michael Niedermayer  wrote:
> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>> This recovers state with #7374 linked sample.
>>
>> Work funded by Open Broadcast Systems.
>>
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavcodec/h264_refs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> index eaf965e43d..5645a203a7 100644
>> --- a/libavcodec/h264_refs.c
>> +++ b/libavcodec/h264_refs.c
>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>  }
>>  break;
>>  case MMCO_RESET:
>> +default:
>>  while (h->short_ref_count) {
>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>>  }
>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>>  h->last_pocs[j] = INT_MIN;
>>  break;
>> -default: assert(0);
>>  }
>>  }
>
> mmco[i].opcode should not be invalid, its checked around the point where
> this
> array is filled.
> unless there is something iam missing

Yes, you are missing big time.
If you think by "checked" about those nice asserts they are not enabled at all.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH, v2] lavc/qsvenc: replace assert with error return

2018-12-07 Thread Li, Zhong
> > > Subject: [FFmpeg-devel] [PATCH, v2] lavc/qsvenc: replace assert with
> > > error return
> > >
> > > bs->FrameType is not set in MSDK in some cases (mjpeg encode for
> > > bs->example),
> > > and assert on a value coming from an external library is not proper.
> > >
> > > Add default type check for bs->FrameType, and return invalid data
> > > error in function ff_qsv_encode to avoid using uninitialized value.
> > >
> > > Fix #7593.
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > > [v2]: Add default bs->FrameType check.
> > >
> > >  libavcodec/qsvenc.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > > 7f4592f878..917344b60c 100644
> > > --- a/libavcodec/qsvenc.c
> > > +++ b/libavcodec/qsvenc.c
> > > @@ -1337,8 +1337,10 @@ int ff_qsv_encode(AVCodecContext *avctx,
> > > QSVEncContext *q,
> > >  pict_type = AV_PICTURE_TYPE_P;
> > >  else if (bs->FrameType & MFX_FRAMETYPE_B ||
> bs->FrameType &
> > > MFX_FRAMETYPE_xB)
> > >  pict_type = AV_PICTURE_TYPE_B;
> > > +else if (bs->FrameType == MFX_FRAMETYPE_UNKNOWN)
> >
> > Unknown frame type has potential risk and would better give a waring
> > log message here.
> 
> Thanks, will add.
> 
> >
> > > +pict_type = AV_PICTURE_TYPE_NONE;
> > >  else
> > > -av_assert0(!"Uninitialized pict_type!");
> > > +return AVERROR_INVALIDDATA;
> >
> > If frame type is MFX_FRAMETYPE_IDR or MFX_FRAMETYPE_xIDR, will go
> here
> > but this is not invalid data.
> > So pict_tpye should be set to AV_PICTURE_TYPE_I in this case I believe.
> 
> It seems that  Frame types such as MFX_FRAMETYPE_IDR and
> MFX_FRAMETYPE_xIDR will be set along with MFX_FRAMETYPE_I in MSDK.
> But frame type which only contains IDR or xIDR should also be set to type I.

Should be more robust in ffmpeg level since there are set as different mask 
bits in MSDK API. 
I've sent a separated patch to handle IDR frames.
(It is also aligned with nvenc: 
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/nvenc.c#L1852)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Michael Niedermayer
On Fri, Dec 07, 2018 at 10:28:09AM +0100, Paul B Mahol wrote:
> On 12/7/18, Michael Niedermayer  wrote:
> > On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
> >> Signed-off-by: Paul B Mahol 
> >> ---
> >>  libavcodec/proresdec2.c | 51 ++---
> >>  1 file changed, 27 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> >> index 8581d797fb..80a76bbba1 100644
> >> --- a/libavcodec/proresdec2.c
> >> +++ b/libavcodec/proresdec2.c
> >> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
> >> *avctx)
> >>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
> >>
> >> avctx->bits_per_raw_sample = 10;
> >>
> >>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
> >> switch (avctx->codec_tag) {
> >> case MKTAG('a','p','c','o'):
> >> avctx->profile = FF_PROFILE_PRORES_PROXY;
> >>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
> >> *avctx)
> >> break;
> >> case MKTAG('a','p','4','h'):
> >> avctx->profile = FF_PROFILE_PRORES_;
> >>-avctx->bits_per_raw_sample = 12;
> >> break;
> >> case MKTAG('a','p','4','x'):
> >> avctx->profile = FF_PROFILE_PRORES_XQ;
> >>-avctx->bits_per_raw_sample = 12;
> >> break;
> >> default:
> >>-avctx->profile = FF_PROFILE_UNKNOWN;
> >> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile %d\n",
> >> avctx->codec_tag);
> >> }
> >>+}
> >>+
> >>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
> >>+avctx->profile == FF_PROFILE_PRORES_)
> >>+avctx->bits_per_raw_sample = 12;
> >
> > with this it would be possible to have 12bit output while the profile
> > is set to one having 10bits and vice versa ?
> 
> No, and neither with previous code.
> 
> > maybe the profile should only be left if it is compatible with the
> > decoder output
> 
> I do not follow.

I may have misunderstood the intend of the patch
please document what this fixes in the commit message.

AVCodecContext.profile is for decoding set by the decoder. 
(thats how its documented in avcodec.h)

So the obvious thought that this is about not overriding a profile
set by the user(app) or demuxer might have been flawed on my side
as that seems not possible in the API as it is documented

thx

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] Fix for KLV in mpegts

2018-12-07 Thread Artyom Lebedev

This fixes bug which prevents from proper muxing-in KLV stream into mpeg-ts.

mpegtsenc.c:1526

    char *side_data = NULL;
    int stream_id = -1;

    side_data = av_packet_get_side_data(pkt,
AV_PKT_DATA_MPEGTS_STREAM_ID,
    &side_data_size);
    if (side_data)
    stream_id = side_data[0];

One-byte stream ID is read from "char *" array to integer making it to 
sign-extend which is not correct. Although it writes it correctly to 
stream, since it is treated as one byte again, but it fails with some 
condition checks, e.g. in mpegtsenc.c:1278:


if (stream_id == 0xbd) /* asynchronous KLV */
    pts = dts = AV_NOPTS_VALUE;
stream_id value in such case is 0xffbd.

Fix should be changing side_data type from "char *" to "uint8_t *".


>From d063568a1765f40a79c6dcf44a444e83e0eb0bed Mon Sep 17 00:00:00 2001
From: 
Date: Fri, 7 Dec 2018 11:48:28 +0200
Subject: [PATCH] Fix bug in mpegts muxer which affects KLV async stream
 generation.

---
 libavformat/mpegtsenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 3339e26..4470b71 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -1523,7 +1523,7 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
 int64_t dts = pkt->dts, pts = pkt->pts;
 int opus_samples = 0;
 int side_data_size;
-char *side_data = NULL;
+uint8_t *side_data = NULL;
 int stream_id = -1;
 
 side_data = av_packet_get_side_data(pkt,
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH, v3] lavc/qsvenc: replace assert with error return

2018-12-07 Thread Linjie Fu
bs->FrameType is not set in MSDK in some cases (mjpeg encode for example),
and assert on a value coming from an external library is not proper.

Add default type check for bs->FrameType, and return invalid data error in 
function
ff_qsv_encode to avoid using uninitialized value.

Fix #7593.

Signed-off-by: Linjie Fu 
---
[v2]: Add default bs->FrameType check.
[v3]: Add log message.

 libavcodec/qsvenc.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 7f4592f878..96ffe8a639 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -1337,8 +1337,14 @@ int ff_qsv_encode(AVCodecContext *avctx, QSVEncContext 
*q,
 pict_type = AV_PICTURE_TYPE_P;
 else if (bs->FrameType & MFX_FRAMETYPE_B || bs->FrameType & 
MFX_FRAMETYPE_xB)
 pict_type = AV_PICTURE_TYPE_B;
-else
-av_assert0(!"Uninitialized pict_type!");
+else if (bs->FrameType == MFX_FRAMETYPE_UNKNOWN) {
+pict_type = AV_PICTURE_TYPE_NONE;
+av_log(avctx, AV_LOG_WARNING, "Unkown FrameType, set pict_type to 
AV_PICTURE_TYPE_NONE.\n");
+}
+else {
+av_log(avctx, AV_LOG_ERROR, "Invalid FrameType:%d.\n", 
bs->FrameType);
+return AVERROR_INVALIDDATA;
+}
 
 #if FF_API_CODED_FRAME
 FF_DISABLE_DEPRECATION_WARNINGS
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Paul B Mahol
On 12/7/18, Michael Niedermayer  wrote:
> On Fri, Dec 07, 2018 at 10:28:09AM +0100, Paul B Mahol wrote:
>> On 12/7/18, Michael Niedermayer  wrote:
>> > On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
>> >> Signed-off-by: Paul B Mahol 
>> >> ---
>> >>  libavcodec/proresdec2.c | 51
>> >> ++---
>> >>  1 file changed, 27 insertions(+), 24 deletions(-)
>> >>
>> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
>> >> index 8581d797fb..80a76bbba1 100644
>> >> --- a/libavcodec/proresdec2.c
>> >> +++ b/libavcodec/proresdec2.c
>> >> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
>> >> *avctx)
>> >>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext
>> >> *avctx)
>> >>
>> >> avctx->bits_per_raw_sample = 10;
>> >>
>> >>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
>> >> switch (avctx->codec_tag) {
>> >> case MKTAG('a','p','c','o'):
>> >> avctx->profile = FF_PROFILE_PRORES_PROXY;
>> >>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
>> >> *avctx)
>> >> break;
>> >> case MKTAG('a','p','4','h'):
>> >> avctx->profile = FF_PROFILE_PRORES_;
>> >>-avctx->bits_per_raw_sample = 12;
>> >> break;
>> >> case MKTAG('a','p','4','x'):
>> >> avctx->profile = FF_PROFILE_PRORES_XQ;
>> >>-avctx->bits_per_raw_sample = 12;
>> >> break;
>> >> default:
>> >>-avctx->profile = FF_PROFILE_UNKNOWN;
>> >> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile
>> >> %d\n",
>> >> avctx->codec_tag);
>> >> }
>> >>+}
>> >>+
>> >>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
>> >>+avctx->profile == FF_PROFILE_PRORES_)
>> >>+avctx->bits_per_raw_sample = 12;
>> >
>> > with this it would be possible to have 12bit output while the profile
>> > is set to one having 10bits and vice versa ?
>>
>> No, and neither with previous code.
>>
>> > maybe the profile should only be left if it is compatible with the
>> > decoder output
>>
>> I do not follow.
>
> I may have misunderstood the intend of the patch
> please document what this fixes in the commit message.
>
> AVCodecContext.profile is for decoding set by the decoder.
> (thats how its documented in avcodec.h)
>
> So the obvious thought that this is about not overriding a profile
> set by the user(app) or demuxer might have been flawed on my side
> as that seems not possible in the API as it is documented

You missed fact that profile is set via codecpar (which is than copied
to codec context), never in codec context directly.

Once again, what you want to change?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/mov.c: Fix infinite loop when fragments seek

2018-12-07 Thread Seokjin Hong
It will never escape this loop when moof has no sidx info

Signed-off-by: Seokjin Hong 
---
 libavformat/mov.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index ec57a05803..b7b69e2772 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1293,6 +1293,8 @@ static int search_frag_timestamp(MOVFragmentIndex 
*frag_index,
 b = m;
 if (frag_time <= timestamp)
 a = m;
+} else {
+return -1;
 }
 }
 return a;
-- 
2.13.6 (Apple Git-96)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 7/7] avcodec/dpx: fix spotted code style issues

2018-12-07 Thread Paul B Mahol
On 12/6/18, Paul B Mahol  wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/dpx.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>

Will apply this patch set ASAP.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH, v2] lavc/hevc_parser: add 4 bytes startcode condition and update FATE

2018-12-07 Thread Linjie Fu
The startcode before VPS,SPS,PPS and the first NALU in an AU is 4 bytes.
Blindly taking the startcode as 3 bytes will leave 0x00 in last packet
and may lead to some warnings in parse_nal_units when s->flags is set to
PARSER_FLAG_COMPLETE_FRAMES.

Add 4 bytes startcode condition in hevc_find_frame_end.
Modify the code to print the buf_size like in H264 and reduce the duplication.

Update the FATE checksum for fate-hevc-bsf-mp4toannexb:
The ref output has redundant 0x00 at the end of the SUFFIX_SEI:
{ 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41
c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1
12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 }

To verify the output of FATE:
ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags \
+bitexact hevc-mp4.mov
ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out

Signed-off-by: Linjie Fu 
---
[v2]: Update FATE checksum

 libavcodec/hevc_parser.c | 15 ++-
 tests/fate/hevc.mak  |  2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
index 369d1338d0..aa216e3c8d 100644
--- a/libavcodec/hevc_parser.c
+++ b/libavcodec/hevc_parser.c
@@ -32,6 +32,7 @@
 #include "parser.h"
 
 #define START_CODE 0x01 ///< start_code_prefix_one_3bytes
+#define START_CODE_4 0x0001 ///< start_code_4bytes
 
 #define IS_IRAP_NAL(nal) (nal->type >= 16 && nal->type <= 23)
 #define IS_IDR_NAL(nal) (nal->type == HEVC_NAL_IDR_W_RADL || nal->type == 
HEVC_NAL_IDR_N_LP)
@@ -239,7 +240,7 @@ static int parse_nal_units(AVCodecParserContext *s, const 
uint8_t *buf,
 }
 }
 /* didn't find a picture! */
-av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n");
+av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with size 
%d\n", buf_size);
 return -1;
 }
 
@@ -267,8 +268,7 @@ static int hevc_find_frame_end(AVCodecParserContext *s, 
const uint8_t *buf,
 if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == 
HEVC_NAL_SEI_PREFIX ||
 (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) {
 if (pc->frame_start_found) {
-pc->frame_start_found = 0;
-return i - 5;
+goto found;
 }
 } else if (nut <= HEVC_NAL_RASL_R ||
(nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) {
@@ -277,14 +277,19 @@ static int hevc_find_frame_end(AVCodecParserContext *s, 
const uint8_t *buf,
 if (!pc->frame_start_found) {
 pc->frame_start_found = 1;
 } else { // First slice of next frame found
-pc->frame_start_found = 0;
-return i - 5;
+goto found;
 }
 }
 }
 }
 
 return END_NOT_FOUND;
+
+found:
+pc->frame_start_found = 0;
+if (((pc->state64 >> 3 * 8) & 0x) == START_CODE_4)
+return i - 6;
+return i - 5;
 }
 
 static int hevc_parse(AVCodecParserContext *s, AVCodecContext *avctx,
diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index db3ea19340..ab8da53535 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -238,7 +238,7 @@ FATE_HEVC-$(call ALLYES, HEVC_DEMUXER MOV_DEMUXER 
HEVC_MP4TOANNEXB_BSF MOV_MUXER
 fate-hevc-bsf-mp4toannexb: tests/data/hevc-mp4.mov
 fate-hevc-bsf-mp4toannexb: CMD = md5 -i $(TARGET_PATH)/tests/data/hevc-mp4.mov 
-c:v copy -fflags +bitexact -f hevc
 fate-hevc-bsf-mp4toannexb: CMP = oneline
-fate-hevc-bsf-mp4toannexb: REF = 1873662a3af1848c37e4eb25722c8df9
+fate-hevc-bsf-mp4toannexb: REF = 73019329ed7f81c24f9af67c34c640c0
 
 fate-hevc-skiploopfilter: CMD = framemd5 -skip_loop_filter nokey -i 
$(TARGET_SAMPLES)/hevc-conformance/SAO_D_Samsung_5.bit -sws_flags bitexact
 FATE_HEVC += fate-hevc-skiploopfilter
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes startcode condition in hevc_find_frame_end

2018-12-07 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Friday, November 30, 2018 07:01
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes
> startcode condition in hevc_find_frame_end
> 
> On Thu, Nov 29, 2018 at 04:14:29AM +, Fu, Linjie wrote:
> > > -Original Message-
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > > Of Michael Niedermayer
> > > Sent: Thursday, November 29, 2018 02:14
> > > To: FFmpeg development discussions and patches  > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH] lavc/hevc_parser: add 4 bytes
> > > startcode condition in hevc_find_frame_end
> > >
> > > On Tue, Nov 27, 2018 at 08:16:55PM +0800, Linjie Fu wrote:
> > > > The startcode before VPS,SPS,PPS and the first NALU in an AU is 4 bytes.
> > > > Blindly taking the startcode as 3 bytes will leave 0x00 in last packet
> > > > and may lead to some warnings in parse_nal_units when s->flags is set
> to
> > > > PARSER_FLAG_COMPLETE_FRAMES.
> > > >
> > > > Add 4 bytes startcode condition in hevc_find_frame_end.
> > > > Modify the code to print the buf_size like in H264 and reduce the
> > > duplication.
> > > >
> > > > Signed-off-by: Linjie Fu 
> > > > ---
> > > >  libavcodec/hevc_parser.c | 15 ++-
> > > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > >
> > > breaks
> > >  make fate-hevc-bsf-mp4toannexb
> > > TESThevc-bsf-mp4toannexb
> > > --- - 2018-11-28 19:12:47.869732022 +0100
> > > +++ tests/data/fate/hevc-bsf-mp4toannexb  2018-11-28
> > > 19:12:47.864276885 +0100
> > > @@ -1 +1 @@
> > > -1873662a3af1848c37e4eb25722c8df9
> > > +73019329ed7f81c24f9af67c34c640c0
> > > Test hevc-bsf-mp4toannexb failed. Look at tests/data/fate/hevc-bsf-
> > > mp4toannexb.err for details.
> > > make: *** [fate-hevc-bsf-mp4toannexb] Error 1
> > >
> >
> > Investigate the root cause:
> >
> > In fate test:
> > Step 1. ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c
> copy -flags +bitexact hevc-mp4.mov
> > Step 2. ffmpeg -i hevc-mp4_after.mov -c:v copy -fflags +bitexact -f hevc
> hevc.out
> > Step3. md5sum and compared with the reference
> >
> > Compare the output file in Step 1 before and after the patch, and found it
> changed:
> > -rw-r--r-- 1 root root 68460 11月 29 11:23 hevc-mp4_after.mov
> > -rw-r--r-- 1 root root 68507 11月 29 11:31 hevc-mp4_before.mov
> >
> > Compare the original file with two output files:
> > SUFFIX_SEI bit information in  the first frame for example :
> >
> > WPP_A_ericsson_MAIN10_2.bit: { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09
> 38 9a 79 41 c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 12 72 67 
> d6 f0
> 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 }
> >
> > hevc-mp4_before.mov: { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79
> 41 c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 12 72 67 d6 f0 8f 
> 66 ee
> 95 f9 e2 b9 ba 23 9a e8 80 00 }
> >
> > hevc-mp4_after.mov: { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41
> c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1 12 72 67 d6 f0 8f 66 
> ee 95
> f9 e2 b9 ba 23 9a e8 80 }
> >
> > Before applying the patch, the output file has an extra 0x00 at the end of
> SUFFIX_SEI (because of blindly taking startcode as 3 bytes and left one byte).
> > After applying the patch, the output file has the same bit info like the
> original file.
> > So, I think this patch works.
> 
> if the patch break make fate then it doesnt work.
> If a change cause by a patch is correct it needs to update the
> test / its checksum
> This is also important so that anyone looking at the patch knows which tests
> change and anyone testing knows how to detect a test failure vs a passed
> test

Thanks, sent a [v2] patch with the updated FATE checksum.
---
Linjie

> [...]
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Those who are best at talking, realize last or never when they are wrong.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH V3 2/2] doc: Add libsvt_hevc encoder docs

2018-12-07 Thread Jun Zhao
Signed-off-by: Jun Zhao 
Signed-off-by: Huang, Zhengxu 
Signed-off-by: hassene 
---
 doc/encoders.texi |   98 +
 doc/general.texi  |8 
 2 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 4db7764..33efbef 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1566,6 +1566,104 @@ Set maximum NAL size in bytes.
 Allow skipping frames to hit the target bitrate if set to 1.
 @end table
 
+@section libsvt_hevc
+
+Intel Scalable Video Technology HEVC encoder wrapper.
+
+This encoder requires the presence of the headers and
+library during configuration. You need to explicitly configure the
+build with @code{--enable-libsvt}. The library is detected using
+@command{pkg-config}.
+
+For more information about the library see
+@url{https://github.com/intel/SVT-HEVC.git}.
+
+@subsection Options
+
+The following FFmpeg global options affect the configurations of the
+libsvt_hevc encoder.
+
+@table @option
+@item vui
+Enables or disables the vui structure in the HEVC elementary
+bitstream. 0 = Off, 1 = On
+
+@item hielevel
+Set hierarchical levels. Can assume one of the following possible values:
+
+@table @samp
+@item flat
+none hierarchy level
+@item 2level
+2-level hierarchy
+@item 3level
+3-level hierarchy
+@item 4level
+4-level hierarchy
+@end table
+
+@item la_depth
+Set look-ahead depth, depending on bit rate control mode @option{rc}, When
+bit rate control mode is set to vbr it's best to set this parameter to be
+equal to the Intra period value (such is the default set by the encoder),
+when cqp is chosen, then a look ahead is recommended.
+
+@item intra_ref_type
+Set intra refesh type. Can assume one of the following possible values:
+
+@table @samp
+@item cra
+open group of pictures
+@item idr
+closed group of pictures
+@end table
+
+@item preset
+A preset defining the quality vs density tradeoff point that the
+encoding is to be performed at.(e.g. 0 is the highest quality mode,
+12 is the highest density mode).
+
+@item profile (@emph{profile})
+Set profile restrictions. Can assume one of the following possible values:
+
+@table @samp
+@item main
+main profile
+@item main10
+main10 profile
+@end table
+
+@item rc
+Set bit rate control mode. Can assume one of the following possible values:
+
+@table @samp
+@item cqp
+cqp mode
+@item vbr
+vbr mode
+@end table
+
+@item qp
+Initial quantization parameter for the intra pictures used when
+@option{rc} is cqp mode.
+
+@item sc_detection
+Enables or disables the scene change detection algorithm.
+
+@item tune
+Set quality mode. Can assume one of the following possible values:
+
+@table @samp
+@item sq
+subjective quality mode
+@item oq
+objective quality mode
+@end table
+
+@item bl_mode
+Enables or disables Random Access Prediction.
+@end table
+
 @section libtheora
 
 libtheora Theora encoder wrapper.
diff --git a/doc/general.texi b/doc/general.texi
index d883240..a071bfd 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -196,6 +196,14 @@ constrained baseline profile and CABAC.) Using it is 
mostly useful for
 testing and for taking advantage of Cisco's patent portfolio license
 (@url{http://www.openh264.org/BINARY_LICENSE.txt}).
 
+@section SVT-HEVC
+
+FFmpeg can make use of the SVT-HEVC library for HEVC encoding.
+
+Go to @url{https://github.com/intel/SVT-HEVC.git} and follow the instructions
+for installing the library. Then pass @code{--enable-libsvt} to configure to
+enable it.
+
 @section x264
 
 FFmpeg can make use of the x264 library for H.264 encoding.
-- 
1.7.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH V3 0/2] Add libsvt HEVC encoder wrapper

2018-12-07 Thread Jun Zhao
The Scalable Video Technology for HEVC Encoder (SVT-HEVC Encoder) is an 
HEVC-compliant encoder library core that achieves excellent density-quality 
tradeoffs, and is highly optimized for Intel Xeon Scalable Processor and 
Xeon D processors. Intel open source SVT-HEVC encoder in: 
https://github.com/intel/SVT-HEVC.

This wrapper work with SVT-HEVC new_api branch, more information can get 
from https://github.com/intel/SVT-HEVC/blob/new_api/ffmpeg_plugin/.

For SVT-HEVC build, you can switch the branch to new_api, then run:

mkdir build && cd build && cmake ../ && make -j `nproc` && sudo make install
 
in the directory. (Yes, SVT-HEVC now support the full CMake style's 
build/install 
after the commit 
https://github.com/intel/SVT-HEVC/commit/f28d492971ef58dc655ccdc7d819cc18ddb45a2f)

This patches based on Zhengxu, huang/hassene's hard work.

More performance/quality data will be public as soon as quick.

V3: - Merge Changelog patch to new wrapper(Tks Moritz/Carl's review)

V2: - Refine the error handle (Tks Steven/James's review).
- Add docs part (Tks Steven/Moritz's review).
- Use named parameters in options.
- Follow FFmpeg coding style and Changelog.

V1: - Add libsvt hevc encoder wrapper and a Changelog entry.

Jun Zhao (2):
  lavc/svt_hevc: add libsvt hevc encoder wrapper.
  doc: Add libsvt_hevc encoder docs

 Changelog|1 +
 configure|4 +
 doc/encoders.texi|   98 ++
 doc/general.texi |8 +
 libavcodec/Makefile  |1 +
 libavcodec/allcodecs.c   |1 +
 libavcodec/libsvt_hevc.c |  440 ++
 7 files changed, 553 insertions(+), 0 deletions(-)
 create mode 100644 libavcodec/libsvt_hevc.c

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH V3 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2018-12-07 Thread Jun Zhao
base on patch by Huang, Zhengxu from https://github.com/intel/SVT-HEVC

Signed-off-by: Huang, Zhengxu 
Signed-off-by: hassene 
Signed-off-by: Jun Zhao 
---
 Changelog|1 +
 configure|4 +
 libavcodec/Makefile  |1 +
 libavcodec/allcodecs.c   |1 +
 libavcodec/libsvt_hevc.c |  440 ++
 5 files changed, 447 insertions(+), 0 deletions(-)
 create mode 100644 libavcodec/libsvt_hevc.c

diff --git a/Changelog b/Changelog
index 1f53ff4..cef444d 100644
--- a/Changelog
+++ b/Changelog
@@ -10,6 +10,7 @@ version :
 - truehd_core bitstream filter
 - dhav demuxer
 - PCM-DVD encoder
+- add SVT(Scalable Video Technology) HEVC encoder
 
 
 version 4.1:
diff --git a/configure b/configure
index 642c13d..32bb97e 100755
--- a/configure
+++ b/configure
@@ -263,6 +263,7 @@ External library support:
   --enable-libspeexenable Speex de/encoding via libspeex [no]
   --enable-libsrt  enable Haivision SRT protocol via libsrt [no]
   --enable-libssh  enable SFTP protocol via libssh [no]
+  --enable-libsvt  enable HEVC encoding via svt [no]
   --enable-libtensorflow   enable TensorFlow as a DNN module backend
for DNN based filters like sr [no]
   --enable-libtesseractenable Tesseract, needed for ocr filter [no]
@@ -1665,6 +1666,7 @@ EXTERNAL_LIBRARY_GPL_LIST="
 libcdio
 libdavs2
 librubberband
+libsvt
 libvidstab
 libx264
 libx265
@@ -3130,6 +3132,7 @@ libshine_encoder_select="audio_frame_queue"
 libspeex_decoder_deps="libspeex"
 libspeex_encoder_deps="libspeex"
 libspeex_encoder_select="audio_frame_queue"
+libsvt_hevc_encoder_deps="libsvt"
 libtheora_encoder_deps="libtheora"
 libtwolame_encoder_deps="libtwolame"
 libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
@@ -6148,6 +6151,7 @@ enabled libsoxr   && require libsoxr soxr.h 
soxr_create -lsoxr
 enabled libssh&& require_pkg_config libssh libssh libssh/sftp.h 
sftp_init
 enabled libspeex  && require_pkg_config libspeex speex speex/speex.h 
speex_decoder_init
 enabled libsrt&& require_pkg_config libsrt "srt >= 1.3.0" 
srt/srt.h srt_socket
+enabled libsvt&& require_pkg_config libsvt  libsvt  EbApi.h 
EbInitHandle
 enabled libtensorflow && require libtensorflow tensorflow/c/c_api.h 
TF_Version -ltensorflow
 enabled libtesseract  && require_pkg_config libtesseract tesseract 
tesseract/capi.h TessBaseAPICreate
 enabled libtheora && require libtheora theora/theoraenc.h th_info_init 
-ltheoraenc -ltheoradec -logg
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 5feadac..1a2506a 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -983,6 +983,7 @@ OBJS-$(CONFIG_LIBOPUS_ENCODER)+= libopusenc.o 
libopus.o \
 OBJS-$(CONFIG_LIBSHINE_ENCODER)   += libshine.o
 OBJS-$(CONFIG_LIBSPEEX_DECODER)   += libspeexdec.o
 OBJS-$(CONFIG_LIBSPEEX_ENCODER)   += libspeexenc.o
+OBJS-$(CONFIG_LIBSVT_HEVC_ENCODER)+= libsvt_hevc.o
 OBJS-$(CONFIG_LIBTHEORA_ENCODER)  += libtheoraenc.o
 OBJS-$(CONFIG_LIBTWOLAME_ENCODER) += libtwolame.o
 OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER) += libvo-amrwbenc.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index d70646e..094ee3c 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -699,6 +699,7 @@ extern AVCodec ff_librsvg_decoder;
 extern AVCodec ff_libshine_encoder;
 extern AVCodec ff_libspeex_encoder;
 extern AVCodec ff_libspeex_decoder;
+extern AVCodec ff_libsvt_hevc_encoder;
 extern AVCodec ff_libtheora_encoder;
 extern AVCodec ff_libtwolame_encoder;
 extern AVCodec ff_libvo_amrwbenc_encoder;
diff --git a/libavcodec/libsvt_hevc.c b/libavcodec/libsvt_hevc.c
new file mode 100644
index 000..6ec9e16
--- /dev/null
+++ b/libavcodec/libsvt_hevc.c
@@ -0,0 +1,440 @@
+/*
+* Scalable Video Technology for HEVC encoder library plugin
+*
+* Copyright (c) 2018 Intel Corporation
+*
+* This program is free software; you can redistribute it and/or
+* modify it under the terms of the GNU Lesser General Public
+* License as published by the Free Software Foundation; either
+* version 2.1 of the License, or (at your option) any later version.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this program; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+*/
+
+#include "EbErrorCodes.h"
+#include "EbTime.h"
+#include "EbApi.h"
+
+#include "libavutil/common.h"
+#include "libavutil/frame.h"
+#include "libavutil/opt.h"
+
+#include "internal.h"
+#include "avcodec.h"
+
+typedef struct Svt

Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Michael Niedermayer
On Fri, Dec 07, 2018 at 11:21:57AM +0100, Paul B Mahol wrote:
> On 12/7/18, Michael Niedermayer  wrote:
> > On Fri, Dec 07, 2018 at 10:28:09AM +0100, Paul B Mahol wrote:
> >> On 12/7/18, Michael Niedermayer  wrote:
> >> > On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
> >> >> Signed-off-by: Paul B Mahol 
> >> >> ---
> >> >>  libavcodec/proresdec2.c | 51
> >> >> ++---
> >> >>  1 file changed, 27 insertions(+), 24 deletions(-)
> >> >>
> >> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> >> >> index 8581d797fb..80a76bbba1 100644
> >> >> --- a/libavcodec/proresdec2.c
> >> >> +++ b/libavcodec/proresdec2.c
> >> >> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
> >> >> *avctx)
> >> >>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext
> >> >> *avctx)
> >> >>
> >> >> avctx->bits_per_raw_sample = 10;
> >> >>
> >> >>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
> >> >> switch (avctx->codec_tag) {
> >> >> case MKTAG('a','p','c','o'):
> >> >> avctx->profile = FF_PROFILE_PRORES_PROXY;
> >> >>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
> >> >> *avctx)
> >> >> break;
> >> >> case MKTAG('a','p','4','h'):
> >> >> avctx->profile = FF_PROFILE_PRORES_;
> >> >>-avctx->bits_per_raw_sample = 12;
> >> >> break;
> >> >> case MKTAG('a','p','4','x'):
> >> >> avctx->profile = FF_PROFILE_PRORES_XQ;
> >> >>-avctx->bits_per_raw_sample = 12;
> >> >> break;
> >> >> default:
> >> >>-avctx->profile = FF_PROFILE_UNKNOWN;
> >> >> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile
> >> >> %d\n",
> >> >> avctx->codec_tag);
> >> >> }
> >> >>+}
> >> >>+
> >> >>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
> >> >>+avctx->profile == FF_PROFILE_PRORES_)
> >> >>+avctx->bits_per_raw_sample = 12;
> >> >
> >> > with this it would be possible to have 12bit output while the profile
> >> > is set to one having 10bits and vice versa ?
> >>
> >> No, and neither with previous code.
> >>
> >> > maybe the profile should only be left if it is compatible with the
> >> > decoder output
> >>
> >> I do not follow.
> >
> > I may have misunderstood the intend of the patch
> > please document what this fixes in the commit message.
> >
> > AVCodecContext.profile is for decoding set by the decoder.
> > (thats how its documented in avcodec.h)
> >
> > So the obvious thought that this is about not overriding a profile
> > set by the user(app) or demuxer might have been flawed on my side
> > as that seems not possible in the API as it is documented
> 
> You missed fact that profile is set via codecpar (which is than copied
> to codec context), never in codec context directly.
> 
> Once again, what you want to change?

As i said, please document in the commit message what this fixes.

About codecpar, The documentation of the codec context did not allow
code outside the decoder to set profile and it now is set from outside
the decoder. That broadening of the interpretation is at least a source
for potential bugs.

So, if i guess correctly, the issue this is about is that
codecpar has a profile set and that is given to
the decoder which then previously did override it and after the patch does
sometimes not. 
So my original concern was that the value set in codecpar can be incorrect,
this value could come from the user application, demuxer or other source.

As a specific example, the profile might be set to FF_PROFILE_PRORES_PROXY
That IIUC corresponds to "Apple ProRes 422 Proxy" in apples documents
and now the decoder could output 12bit 444 without correcting the profile.
IIUC this would be inconsistent

This is not a major issue, its just metadata thats contradictionary

Another minor issue is that this behavior is undocumented, or incorrectly
documented

For example for width and height we document in avcodec.h:
 * - decoding: May be set by the user before opening the decoder if known 
e.g.
 * from the container. Some decoders will require the dimensions
 * to be set by the caller. During decoding, the decoder may
 * overwrite those values as required while parsing the data.
 */
int width, height;

That says clearly that the user can set them and that they will be overriden but
with profile we have:

 * - decoding: Set by libavcodec.
 */
 int profile;

Before this patch this was correct for prores, after the patch this 
API documentation is at least misleading or incomplete
The decoder not just sometimes leaves the field but it sometimes also reads the
field and uses it for the bits_per_raw_sample setting.

What i want is to keep this all consistent and have documentation match
implementation. And things being documented well enough to use them based
on just the documentation

Thank

Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Paul B Mahol
On 12/7/18, Michael Niedermayer  wrote:
> On Fri, Dec 07, 2018 at 11:21:57AM +0100, Paul B Mahol wrote:
>> On 12/7/18, Michael Niedermayer  wrote:
>> > On Fri, Dec 07, 2018 at 10:28:09AM +0100, Paul B Mahol wrote:
>> >> On 12/7/18, Michael Niedermayer  wrote:
>> >> > On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
>> >> >> Signed-off-by: Paul B Mahol 
>> >> >> ---
>> >> >>  libavcodec/proresdec2.c | 51
>> >> >> ++---
>> >> >>  1 file changed, 27 insertions(+), 24 deletions(-)
>> >> >>
>> >> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
>> >> >> index 8581d797fb..80a76bbba1 100644
>> >> >> --- a/libavcodec/proresdec2.c
>> >> >> +++ b/libavcodec/proresdec2.c
>> >> >> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
>> >> >> *avctx)
>> >> >>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext
>> >> >> *avctx)
>> >> >>
>> >> >> avctx->bits_per_raw_sample = 10;
>> >> >>
>> >> >>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
>> >> >> switch (avctx->codec_tag) {
>> >> >> case MKTAG('a','p','c','o'):
>> >> >> avctx->profile = FF_PROFILE_PRORES_PROXY;
>> >> >>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
>> >> >> *avctx)
>> >> >> break;
>> >> >> case MKTAG('a','p','4','h'):
>> >> >> avctx->profile = FF_PROFILE_PRORES_;
>> >> >>-avctx->bits_per_raw_sample = 12;
>> >> >> break;
>> >> >> case MKTAG('a','p','4','x'):
>> >> >> avctx->profile = FF_PROFILE_PRORES_XQ;
>> >> >>-avctx->bits_per_raw_sample = 12;
>> >> >> break;
>> >> >> default:
>> >> >>-avctx->profile = FF_PROFILE_UNKNOWN;
>> >> >> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile
>> >> >> %d\n",
>> >> >> avctx->codec_tag);
>> >> >> }
>> >> >>+}
>> >> >>+
>> >> >>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
>> >> >>+avctx->profile == FF_PROFILE_PRORES_)
>> >> >>+avctx->bits_per_raw_sample = 12;
>> >> >
>> >> > with this it would be possible to have 12bit output while the
>> >> > profile
>> >> > is set to one having 10bits and vice versa ?
>> >>
>> >> No, and neither with previous code.
>> >>
>> >> > maybe the profile should only be left if it is compatible with the
>> >> > decoder output
>> >>
>> >> I do not follow.
>> >
>> > I may have misunderstood the intend of the patch
>> > please document what this fixes in the commit message.
>> >
>> > AVCodecContext.profile is for decoding set by the decoder.
>> > (thats how its documented in avcodec.h)
>> >
>> > So the obvious thought that this is about not overriding a profile
>> > set by the user(app) or demuxer might have been flawed on my side
>> > as that seems not possible in the API as it is documented
>>
>> You missed fact that profile is set via codecpar (which is than copied
>> to codec context), never in codec context directly.
>>
>> Once again, what you want to change?
>
> As i said, please document in the commit message what this fixes.
>
> About codecpar, The documentation of the codec context did not allow
> code outside the decoder to set profile and it now is set from outside
> the decoder. That broadening of the interpretation is at least a source
> for potential bugs.
>
> So, if i guess correctly, the issue this is about is that
> codecpar has a profile set and that is given to
> the decoder which then previously did override it and after the patch does
> sometimes not.
> So my original concern was that the value set in codecpar can be incorrect,
> this value could come from the user application, demuxer or other source.
>
> As a specific example, the profile might be set to FF_PROFILE_PRORES_PROXY
> That IIUC corresponds to "Apple ProRes 422 Proxy" in apples documents
> and now the decoder could output 12bit 444 without correcting the profile.
> IIUC this would be inconsistent
>
> This is not a major issue, its just metadata thats contradictionary
>
> Another minor issue is that this behavior is undocumented, or incorrectly
> documented
>
> For example for width and height we document in avcodec.h:
>  * - decoding: May be set by the user before opening the decoder if
> known e.g.
>  * from the container. Some decoders will require the
> dimensions
>  * to be set by the caller. During decoding, the decoder
> may
>  * overwrite those values as required while parsing the
> data.
>  */
> int width, height;
>
> That says clearly that the user can set them and that they will be overriden
> but
> with profile we have:
>
>  * - decoding: Set by libavcodec.
>  */
>  int profile;
>
> Before this patch this was correct for prores, after the patch this
> API documentation is at least misleading or incomplete
> The decoder not just sometimes leaves the field but it sometimes also reads
> the
> field and uses it for the b

Re: [FFmpeg-devel] Fix for KLV in mpegts

2018-12-07 Thread Peter Ross
On Fri, Dec 07, 2018 at 12:06:36PM +0200, Artyom Lebedev wrote:
> This fixes bug which prevents from proper muxing-in KLV stream into mpeg-ts.
> 
> mpegtsenc.c:1526
> 
>     char *side_data = NULL;
>     int stream_id = -1;
> 
>     side_data = av_packet_get_side_data(pkt,
> AV_PKT_DATA_MPEGTS_STREAM_ID,
>     &side_data_size);
>     if (side_data)
>     stream_id = side_data[0];
> 
> One-byte stream ID is read from "char *" array to integer making it to
> sign-extend which is not correct. Although it writes it correctly to stream,
> since it is treated as one byte again, but it fails with some condition
> checks, e.g. in mpegtsenc.c:1278:
> 
> if (stream_id == 0xbd) /* asynchronous KLV */
>     pts = dts = AV_NOPTS_VALUE;
> stream_id value in such case is 0xffbd.
> 
> Fix should be changing side_data type from "char *" to "uint8_t *".

looks good. i will apply if no one else rejects.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V3 2/2] doc: Add libsvt_hevc encoder docs

2018-12-07 Thread Gyan Doshi
You'll need to rebase as I just rearranged entries (and style, a bit) in 
doc/general


On 07-12-2018 05:26 PM, Jun Zhao wrote:

Signed-off-by: Jun Zhao 
Signed-off-by: Huang, Zhengxu 
Signed-off-by: hassene 
---
  doc/encoders.texi |   98 +
  doc/general.texi  |8 
  2 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 4db7764..33efbef 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1566,6 +1566,104 @@ Set maximum NAL size in bytes.
  Allow skipping frames to hit the target bitrate if set to 1.
  @end table
  
+@section libsvt_hevc



+Intel Scalable Video Technology HEVC encoder wrapper.
+
+This encoder requires the presence of the headers and
+library during configuration. You need to explicitly configure the
+build with @code{--enable-libsvt}. The library is detected using
+@command{pkg-config}.
+
+For more information about the library see
+@url{https://github.com/intel/SVT-HEVC.git}.
+
+@subsection Options
+
+The following FFmpeg global options affect the configurations of the
+libsvt_hevc encoder.



All of the below appear to be private options declared in the wrapper.



+
+@table @option
+@item vui
+Enables or disables the vui structure in the HEVC elementary
+bitstream. 0 = Off, 1 = On
+
+@item hielevel
+Set hierarchical levels. Can assume one of the following possible values:
+
+@table @samp
+@item flat
+none hierarchy level
+@item 2level
+2-level hierarchy
+@item 3level
+3-level hierarchy
+@item 4level
+4-level hierarchy
+@end table
+
+@item la_depth
+Set look-ahead depth, depending on bit rate control mode @option{rc}, When
+bit rate control mode is set to vbr it's best to set this parameter to be
+equal to the Intra period value (such is the default set by the encoder),
+when cqp is chosen, then a look ahead is recommended.
+
+@item intra_ref_type
+Set intra refesh type. Can assume one of the following possible values:
+
+@table @samp
+@item cra
+open group of pictures
+@item idr
+closed group of pictures
+@end table
+
+@item preset
+A preset defining the quality vs density tradeoff point that the
+encoding is to be performed at.(e.g. 0 is the highest quality mode,
+12 is the highest density mode).
+
+@item profile (@emph{profile})
+Set profile restrictions. Can assume one of the following possible values:
+
+@table @samp
+@item main
+main profile
+@item main10
+main10 profile
+@end table
+
+@item rc
+Set bit rate control mode. Can assume one of the following possible values:
+
+@table @samp
+@item cqp
+cqp mode
+@item vbr
+vbr mode
+@end table
+
+@item qp
+Initial quantization parameter for the intra pictures used when
+@option{rc} is cqp mode.
+
+@item sc_detection
+Enables or disables the scene change detection algorithm.
+
+@item tune
+Set quality mode. Can assume one of the following possible values:
+
+@table @samp
+@item sq
+subjective quality mode
+@item oq
+objective quality mode
+@end table
+
+@item bl_mode
+Enables or disables Random Access Prediction.
+@end table
+
  @section libtheora
  
  libtheora Theora encoder wrapper.

diff --git a/doc/general.texi b/doc/general.texi
index d883240..a071bfd 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -196,6 +196,14 @@ constrained baseline profile and CABAC.) Using it is 
mostly useful for
  testing and for taking advantage of Cisco's patent portfolio license
  (@url{http://www.openh264.org/BINARY_LICENSE.txt}).
  
+@section SVT-HEVC



SVT-HEVC --> Intel SVT-HEVC



+
+FFmpeg can make use of the SVT-HEVC library for HEVC encoding.
+
+Go to @url{https://github.com/intel/SVT-HEVC.git} and follow the instructions
+for installing the library. Then pass @code{--enable-libsvt} to configure to
+enable it.
+
  @section x264
  
  FFmpeg can make use of the x264 library for H.264 encoding.



Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Fix for KLV in mpegts

2018-12-07 Thread Carl Eugen Hoyos
2018-12-07 13:27 GMT+01:00, Peter Ross :
> On Fri, Dec 07, 2018 at 12:06:36PM +0200, Artyom Lebedev wrote:
>> This fixes bug which prevents from proper muxing-in KLV stream into
>> mpeg-ts.
>>
>> mpegtsenc.c:1526
>>
>> char *side_data = NULL;
>> int stream_id = -1;
>>
>> side_data = av_packet_get_side_data(pkt,
>> AV_PKT_DATA_MPEGTS_STREAM_ID,
>> &side_data_size);
>> if (side_data)
>> stream_id = side_data[0];
>>
>> One-byte stream ID is read from "char *" array to integer making it to
>> sign-extend which is not correct. Although it writes it correctly to
>> stream,
>> since it is treated as one byte again, but it fails with some condition
>> checks, e.g. in mpegtsenc.c:1278:
>>
>> if (stream_id == 0xbd) /* asynchronous KLV */
>> pts = dts = AV_NOPTS_VALUE;
>> stream_id value in such case is 0xffbd.
>>
>> Fix should be changing side_data type from "char *" to "uint8_t *".
>
> looks good. i will apply if no one else rejects.

Please mention ticket #7597.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 5/7] avcodec/dpx: improve decoding of 10 bit gray images

2018-12-07 Thread Carl Eugen Hoyos
2018-12-06 20:34 GMT+01:00, Paul B Mahol :

> +temp = *lbuf >> shift & 0x3FF;
> +*lbuf = *lbuf >> 10;
> +
> +return temp;
> +}
> +
>  static uint16_t read10in32(const uint8_t **ptr, uint32_t * lbuf,
>int * n_datum, int is_big, int shift)
>  {
> @@ -385,13 +403,17 @@ static int decode_frame(AVCodecContext *avctx,
>  (uint16_t*)ptr[1],
>  (uint16_t*)ptr[2],
>  (uint16_t*)ptr[3]};
> -int shift = packing == 1 ? 22 : 20;
> +int shift = elements > 1 ? packing == 1 ? 22 : 20 : packing ==
> 1 ? 2 : 0;

I still find this hard to read but if you cannot simplify please commit.

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Hendrik Leppkes
On Fri, Dec 7, 2018 at 1:15 PM Paul B Mahol  wrote:
>
> On 12/7/18, Michael Niedermayer  wrote:
> > On Fri, Dec 07, 2018 at 11:21:57AM +0100, Paul B Mahol wrote:
> >> On 12/7/18, Michael Niedermayer  wrote:
> >> > On Fri, Dec 07, 2018 at 10:28:09AM +0100, Paul B Mahol wrote:
> >> >> On 12/7/18, Michael Niedermayer  wrote:
> >> >> > On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
> >> >> >> Signed-off-by: Paul B Mahol 
> >> >> >> ---
> >> >> >>  libavcodec/proresdec2.c | 51
> >> >> >> ++---
> >> >> >>  1 file changed, 27 insertions(+), 24 deletions(-)
> >> >> >>
> >> >> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> >> >> >> index 8581d797fb..80a76bbba1 100644
> >> >> >> --- a/libavcodec/proresdec2.c
> >> >> >> +++ b/libavcodec/proresdec2.c
> >> >> >> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
> >> >> >> *avctx)
> >> >> >>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext
> >> >> >> *avctx)
> >> >> >>
> >> >> >> avctx->bits_per_raw_sample = 10;
> >> >> >>
> >> >> >>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
> >> >> >> switch (avctx->codec_tag) {
> >> >> >> case MKTAG('a','p','c','o'):
> >> >> >> avctx->profile = FF_PROFILE_PRORES_PROXY;
> >> >> >>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
> >> >> >> *avctx)
> >> >> >> break;
> >> >> >> case MKTAG('a','p','4','h'):
> >> >> >> avctx->profile = FF_PROFILE_PRORES_;
> >> >> >>-avctx->bits_per_raw_sample = 12;
> >> >> >> break;
> >> >> >> case MKTAG('a','p','4','x'):
> >> >> >> avctx->profile = FF_PROFILE_PRORES_XQ;
> >> >> >>-avctx->bits_per_raw_sample = 12;
> >> >> >> break;
> >> >> >> default:
> >> >> >>-avctx->profile = FF_PROFILE_UNKNOWN;
> >> >> >> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile
> >> >> >> %d\n",
> >> >> >> avctx->codec_tag);
> >> >> >> }
> >> >> >>+}
> >> >> >>+
> >> >> >>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
> >> >> >>+avctx->profile == FF_PROFILE_PRORES_)
> >> >> >>+avctx->bits_per_raw_sample = 12;
> >> >> >
> >> >> > with this it would be possible to have 12bit output while the
> >> >> > profile
> >> >> > is set to one having 10bits and vice versa ?
> >> >>
> >> >> No, and neither with previous code.
> >> >>
> >> >> > maybe the profile should only be left if it is compatible with the
> >> >> > decoder output
> >> >>
> >> >> I do not follow.
> >> >
> >> > I may have misunderstood the intend of the patch
> >> > please document what this fixes in the commit message.
> >> >
> >> > AVCodecContext.profile is for decoding set by the decoder.
> >> > (thats how its documented in avcodec.h)
> >> >
> >> > So the obvious thought that this is about not overriding a profile
> >> > set by the user(app) or demuxer might have been flawed on my side
> >> > as that seems not possible in the API as it is documented
> >>
> >> You missed fact that profile is set via codecpar (which is than copied
> >> to codec context), never in codec context directly.
> >>
> >> Once again, what you want to change?
> >
> > As i said, please document in the commit message what this fixes.
> >
> > About codecpar, The documentation of the codec context did not allow
> > code outside the decoder to set profile and it now is set from outside
> > the decoder. That broadening of the interpretation is at least a source
> > for potential bugs.
> >
> > So, if i guess correctly, the issue this is about is that
> > codecpar has a profile set and that is given to
> > the decoder which then previously did override it and after the patch does
> > sometimes not.
> > So my original concern was that the value set in codecpar can be incorrect,
> > this value could come from the user application, demuxer or other source.
> >
> > As a specific example, the profile might be set to FF_PROFILE_PRORES_PROXY
> > That IIUC corresponds to "Apple ProRes 422 Proxy" in apples documents
> > and now the decoder could output 12bit 444 without correcting the profile.
> > IIUC this would be inconsistent
> >
> > This is not a major issue, its just metadata thats contradictionary
> >
> > Another minor issue is that this behavior is undocumented, or incorrectly
> > documented
> >
> > For example for width and height we document in avcodec.h:
> >  * - decoding: May be set by the user before opening the decoder if
> > known e.g.
> >  * from the container. Some decoders will require the
> > dimensions
> >  * to be set by the caller. During decoding, the decoder
> > may
> >  * overwrite those values as required while parsing the
> > data.
> >  */
> > int width, height;
> >
> > That says clearly that the user can set them and that they will be overriden
> > but
> > with profile we have:
> >
> >  * - decoding: Set

Re: [FFmpeg-devel] [PATCH V3 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2018-12-07 Thread Gyan Doshi

On 07-12-2018 05:26 PM, Jun Zhao wrote:


+{"perset", "Encoding preset [0, 12] (e,g, for subjective quality tuning mode and 
>=4k resolution), [0, 10] (for >= 1080p resolution), [0, 9] (for all resolution and modes)",


preset --> preset


+static const AVCodecDefault eb_enc_defaults[] = {
+{ "b", "7M"},
+{ "refs",  "0" },
+{ "g", "64"   },
+{ "flags", "+cgop" },
+{ NULL },
+};


Maybe note these defaults into the docs since these are silently set by 
the wrapper.


Also, in the docs, note the ranges and default values for each option 
where applicable.


Thanks,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] swscale/output: VSX-optimize nbps yuv2plane1

2018-12-07 Thread Lauri Kasanen
On Fri, 7 Dec 2018 13:50:12 +0100
Carl Eugen Hoyos  wrote:

> > Carl Eugen Hoyos  wrote:
> >> 2018-11-27 14:26 GMT+01:00, Lauri Kasanen :
> >> > Fate passes, each format tested with an image to video conversion.
> >> >
> >> > Depends on "swscale/ppc: Move VSX-using code to its own file".
> >>
> >> > Only tested on LE.
> >>
> >> This patch breaks output on BE, tested with fate-v410enc and:
> >> $ ffmpeg -i fate-suite/lena.pnm -pix_fmt yuv420p10 -vcodec ffv1 out.nut
> >
> > Just checking, was that with the !BE guards removed?
> 
> Correct, sorry for being unclear.
> 
> > Otherwise I don't see how it could affect BE?
> 
> Yes.

Okay, so it otherwise didn't affect BE. Can it be applied, or is BE a
requirement? This is a simple function, and I can guess how to change
it, but for future more complex functions I rather don't want to
blindly try.

LE is the common case for newer POWER really, many distros don't even
support BE.

- Lauri
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Carl Eugen Hoyos
2018-12-06 15:26 GMT+01:00, Paul B Mahol :
> This recovers state with #7374 linked sample.
>
> Work funded by Open Broadcast Systems.
>
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/h264_refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> index eaf965e43d..5645a203a7 100644
> --- a/libavcodec/h264_refs.c
> +++ b/libavcodec/h264_refs.c
> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>  }
>  break;
>  case MMCO_RESET:
> +default:
>  while (h->short_ref_count) {
>  remove_short(h, h->short_ref[0]->frame_num, 0);
>  }
> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>  h->last_pocs[j] = INT_MIN;
>  break;
> -default: assert(0);
>  }
>  }
>

Alternative is to avoid an invalid mmco opcode.

Carl Eugen

diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
index eaf965e..c7e64ec 100644
--- a/libavcodec/h264_refs.c
+++ b/libavcodec/h264_refs.c
@@ -875,6 +875,7 @@
 av_log(logctx, AV_LOG_ERROR,
"illegal memory management control operation %d\n",
opcode);
+mmco[i].opcode = MMCO_RESET;
 return -1;
 }
 if (opcode == MMCO_END)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] libstr.c: Fixed rendezvous mode

2018-12-07 Thread Adrian Grzeca
---
 libavformat/libsrt.c | 33 +++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c
index fe3b312151..fced18fb92 100644
--- a/libavformat/libsrt.c
+++ b/libavformat/libsrt.c
@@ -84,6 +84,8 @@ typedef struct SRTContext {
 char *smoother;
 int messageapi;
 SRT_TRANSTYPE transtype;
+   char *localip;
+   char *localport;
 } SRTContext;
 
 #define D AV_OPT_FLAG_DECODING_PARAM
@@ -128,6 +130,8 @@ static const AVOption libsrt_options[] = {
 { "transtype",  "The transmission type for the socket",
 OFFSET(transtype),AV_OPT_TYPE_INT,  { .i64 = 
SRTT_INVALID }, SRTT_LIVE, SRTT_INVALID, .flags = D|E, "transtype" },
 { "live",   NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRTT_LIVE }, 
INT_MIN, INT_MAX, .flags = D|E, "transtype" },
 { "file",   NULL, 0, AV_OPT_TYPE_CONST,  { .i64 = SRTT_FILE }, 
INT_MIN, INT_MAX, .flags = D|E, "transtype" },
+   { "localip", "IP address of network card to use in rendezvous mode 
(Req. in rendezvous mode)", OFFSET(localip),   
AV_OPT_TYPE_STRING,   { .str = NULL },  .flags = D|E },
+   { "localport", "Source port to use in rendezvous mode (Req. in 
rendezvous mode)", OFFSET(localport),   AV_OPT_TYPE_STRING,   { 
.str = NULL },  .flags = D|E },
 { NULL }
 };
 
@@ -355,6 +359,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int 
flags)
 char portstr[10];
 int open_timeout = 500;
 int eid;
+   struct sockaddr_in la;
 
 eid = srt_epoll_create();
 if (eid < 0)
@@ -395,7 +400,25 @@ static int libsrt_setup(URLContext *h, const char *uri, 
int flags)
 }
 
 cur_ai = ai;
-
+   
+   //Fix for rendezvous mode. Not good only for IPv4 but working.
+   if (s->mode == SRT_MODE_RENDEZVOUS) {
+   if(s->localip == NULL || s->localport == NULL) {
+   av_log(h, AV_LOG_ERROR, "Invalid adapter 
configuration\n");
+   return AVERROR(EIO);
+   }
+   av_log(h, AV_LOG_DEBUG , "Adapter options %s:%s\n", s->localip, 
s->localport);
+   
+   int lp = strtol(s->localport, NULL, 10); //TODO: What if 
localport do not contain numers
+   if (lp <= 0 || lp >= 65536) {
+   av_log(h, AV_LOG_ERROR, "Local port missing in uri\n");
+   return AVERROR(EINVAL);
+   }
+   
+   la.sin_family = AF_INET; //TODO: Add support for IPv6
+   la.sin_port = htons(port);
+   la.sin_addr.s_addr = inet_addr(s->localip); //TODO: Check if 
localip is IP of local interface
+   }
  restart:
 
 fd = srt_socket(cur_ai->ai_family, cur_ai->ai_socktype, 0);
@@ -423,7 +446,7 @@ static int libsrt_setup(URLContext *h, const char *uri, int 
flags)
 fd = ret;
 } else {
 if (s->mode == SRT_MODE_RENDEZVOUS) {
-ret = srt_bind(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
+ret = srt_bind(fd, (struct sockaddr *)&la, sizeof(struct 
sockaddr_in));
 if (ret)
 goto fail1;
 }
@@ -579,6 +602,12 @@ static int libsrt_open(URLContext *h, const char *uri, int 
flags)
 } else {
 return AVERROR(EINVAL);
 }
+}
+   if (av_find_info_tag(buf, sizeof(buf), "localip", p)) {
+s->localip = av_strndup(buf, strlen(buf));
+}
+   if (av_find_info_tag(buf, sizeof(buf), "localport", p)) {
+s->localport = av_strndup(buf, strlen(buf));
 }
 }
 return libsrt_setup(h, uri, flags);
-- 
2.19.2.windows.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V3 0/2] Add libsvt HEVC encoder wrapper

2018-12-07 Thread Carl Eugen Hoyos
2018-12-07 12:55 GMT+01:00, Jun Zhao :

> More performance/quality data will be public as soon as quick.

Please either:
Revisit this patch once quality data showing advantages over
exising (GPL) encoders of this new implementation are public.

Or:

Add the following to the commit message:
This patch adds an HEVC encoder that does not need --enable-gpl.
Even in this case, it would be nice to show that the encoder at
least beats non-hevc encoders under LGPL (or BSD).

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH, v3] lavc/qsvenc: replace assert with error return

2018-12-07 Thread Carl Eugen Hoyos
2018-12-07 11:14 GMT+01:00, Linjie Fu :
> bs->FrameType is not set in MSDK in some cases (mjpeg encode for example),
> and assert on a value coming from an external library is not proper.
>
> Add default type check for bs->FrameType, and return invalid data error in
> function
> ff_qsv_encode to avoid using uninitialized value.
>
> Fix #7593.
>
> Signed-off-by: Linjie Fu 
> ---
> [v2]: Add default bs->FrameType check.
> [v3]: Add log message.
>
>  libavcodec/qsvenc.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 7f4592f878..96ffe8a639 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -1337,8 +1337,14 @@ int ff_qsv_encode(AVCodecContext *avctx,
> QSVEncContext *q,
>  pict_type = AV_PICTURE_TYPE_P;
>  else if (bs->FrameType & MFX_FRAMETYPE_B || bs->FrameType &
> MFX_FRAMETYPE_xB)
>  pict_type = AV_PICTURE_TYPE_B;
> -else
> -av_assert0(!"Uninitialized pict_type!");
> +else if (bs->FrameType == MFX_FRAMETYPE_UNKNOWN) {
> +pict_type = AV_PICTURE_TYPE_NONE;
> +av_log(avctx, AV_LOG_WARNING, "Unkown FrameType, set pict_type
> to AV_PICTURE_TYPE_NONE.\n");
> +}
> +else {

Please fix the indentation.

> +av_log(avctx, AV_LOG_ERROR, "Invalid FrameType:%d.\n",
> bs->FrameType);
> +return AVERROR_INVALIDDATA;
> +}

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavc/qsvenc: set pict_type to be I for IDR frames.

2018-12-07 Thread Carl Eugen Hoyos
2018-12-07 10:25 GMT+01:00, Zhong Li :
> Signed-off-by: Zhong Li 
> ---
>  libavcodec/qsvenc.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index aa7f347..8289a32 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -1378,10 +1378,11 @@ int ff_qsv_encode(AVCodecContext *avctx,
> QSVEncContext *q,
>  new_pkt.size = bs->DataLength;
>
>  if (bs->FrameType & MFX_FRAMETYPE_IDR ||
> -bs->FrameType & MFX_FRAMETYPE_xIDR)
> +bs->FrameType & MFX_FRAMETYPE_xIDR) {
>  new_pkt.flags |= AV_PKT_FLAG_KEY;
> -
> -if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType &
> MFX_FRAMETYPE_xI)
> +pict_type = AV_PICTURE_TYPE_I;

> +}
> +else if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType &
> MFX_FRAMETYPE_xI)

Please merge these lines.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Michael Niedermayer
On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
> On 12/7/18, Paul B Mahol  wrote:
> > On 12/7/18, Michael Niedermayer  wrote:
> >> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
> >>> This recovers state with #7374 linked sample.
> >>>
> >>> Work funded by Open Broadcast Systems.
> >>>
> >>> Signed-off-by: Paul B Mahol 
> >>> ---
> >>>  libavcodec/h264_refs.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
> >>> index eaf965e43d..5645a203a7 100644
> >>> --- a/libavcodec/h264_refs.c
> >>> +++ b/libavcodec/h264_refs.c
> >>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
> >>>  }
> >>>  break;
> >>>  case MMCO_RESET:
> >>> +default:
> >>>  while (h->short_ref_count) {
> >>>  remove_short(h, h->short_ref[0]->frame_num, 0);
> >>>  }
> >>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
> >>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
> >>>  h->last_pocs[j] = INT_MIN;
> >>>  break;
> >>> -default: assert(0);
> >>>  }
> >>>  }
> >>
> >> mmco[i].opcode should not be invalid, its checked around the point where
> >> this
> >> array is filled.
> >> unless there is something iam missing
> >
> > Yes, you are missing big time.
> > If you think by "checked" about those nice asserts they are not enabled at
> > all.
> >
> 
> There is check for invalid opcode, but stored invalid opcode is not changed.

Theres no question that we end with a invalid value in the struct, but that
is not intended to be in there. You can see that this is not intended by
simply looking at the variable that holds the number of entries, it is
not written at all in this case.

So for example if this code stores 5 correct looking mmcos and the 6th is
invalid, 6 are in the array but the number of entries is just left where it
was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid value
later doesnt feel ideal.


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] avcodec/proresdec2: change profile only if it is unknown

2018-12-07 Thread Michael Niedermayer
On Fri, Dec 07, 2018 at 01:43:37PM +0100, Hendrik Leppkes wrote:
> On Fri, Dec 7, 2018 at 1:15 PM Paul B Mahol  wrote:
> >
> > On 12/7/18, Michael Niedermayer  wrote:
> > > On Fri, Dec 07, 2018 at 11:21:57AM +0100, Paul B Mahol wrote:
> > >> On 12/7/18, Michael Niedermayer  wrote:
> > >> > On Fri, Dec 07, 2018 at 10:28:09AM +0100, Paul B Mahol wrote:
> > >> >> On 12/7/18, Michael Niedermayer  wrote:
> > >> >> > On Wed, Dec 05, 2018 at 09:22:48PM +0100, Paul B Mahol wrote:
> > >> >> >> Signed-off-by: Paul B Mahol 
> > >> >> >> ---
> > >> >> >>  libavcodec/proresdec2.c | 51
> > >> >> >> ++---
> > >> >> >>  1 file changed, 27 insertions(+), 24 deletions(-)
> > >> >> >>
> > >> >> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> > >> >> >> index 8581d797fb..80a76bbba1 100644
> > >> >> >> --- a/libavcodec/proresdec2.c
> > >> >> >> +++ b/libavcodec/proresdec2.c
> > >> >> >> @@ -140,32 +140,35 @@ static av_cold int decode_init(AVCodecContext
> > >> >> >> *avctx)
> > >> >> >>@@ -140,6 +140,7 @@ static av_cold int decode_init(AVCodecContext
> > >> >> >> *avctx)
> > >> >> >>
> > >> >> >> avctx->bits_per_raw_sample = 10;
> > >> >> >>
> > >> >> >>+if (avctx->profile == FF_PROFILE_UNKNOWN) {
> > >> >> >> switch (avctx->codec_tag) {
> > >> >> >> case MKTAG('a','p','c','o'):
> > >> >> >> avctx->profile = FF_PROFILE_PRORES_PROXY;
> > >> >> >>@@ -155,16 +156,18 @@ static av_cold int decode_init(AVCodecContext
> > >> >> >> *avctx)
> > >> >> >> break;
> > >> >> >> case MKTAG('a','p','4','h'):
> > >> >> >> avctx->profile = FF_PROFILE_PRORES_;
> > >> >> >>-avctx->bits_per_raw_sample = 12;
> > >> >> >> break;
> > >> >> >> case MKTAG('a','p','4','x'):
> > >> >> >> avctx->profile = FF_PROFILE_PRORES_XQ;
> > >> >> >>-avctx->bits_per_raw_sample = 12;
> > >> >> >> break;
> > >> >> >> default:
> > >> >> >>-avctx->profile = FF_PROFILE_UNKNOWN;
> > >> >> >> av_log(avctx, AV_LOG_WARNING, "Unknown prores profile
> > >> >> >> %d\n",
> > >> >> >> avctx->codec_tag);
> > >> >> >> }
> > >> >> >>+}
> > >> >> >>+
> > >> >> >>+if (avctx->profile == FF_PROFILE_PRORES_XQ ||
> > >> >> >>+avctx->profile == FF_PROFILE_PRORES_)
> > >> >> >>+avctx->bits_per_raw_sample = 12;
> > >> >> >
> > >> >> > with this it would be possible to have 12bit output while the
> > >> >> > profile
> > >> >> > is set to one having 10bits and vice versa ?
> > >> >>
> > >> >> No, and neither with previous code.
> > >> >>
> > >> >> > maybe the profile should only be left if it is compatible with the
> > >> >> > decoder output
> > >> >>
> > >> >> I do not follow.
> > >> >
> > >> > I may have misunderstood the intend of the patch
> > >> > please document what this fixes in the commit message.
> > >> >
> > >> > AVCodecContext.profile is for decoding set by the decoder.
> > >> > (thats how its documented in avcodec.h)
> > >> >
> > >> > So the obvious thought that this is about not overriding a profile
> > >> > set by the user(app) or demuxer might have been flawed on my side
> > >> > as that seems not possible in the API as it is documented
> > >>
> > >> You missed fact that profile is set via codecpar (which is than copied
> > >> to codec context), never in codec context directly.
> > >>
> > >> Once again, what you want to change?
> > >
> > > As i said, please document in the commit message what this fixes.
> > >
> > > About codecpar, The documentation of the codec context did not allow
> > > code outside the decoder to set profile and it now is set from outside
> > > the decoder. That broadening of the interpretation is at least a source
> > > for potential bugs.
> > >
> > > So, if i guess correctly, the issue this is about is that
> > > codecpar has a profile set and that is given to
> > > the decoder which then previously did override it and after the patch does
> > > sometimes not.
> > > So my original concern was that the value set in codecpar can be 
> > > incorrect,
> > > this value could come from the user application, demuxer or other source.
> > >
> > > As a specific example, the profile might be set to FF_PROFILE_PRORES_PROXY
> > > That IIUC corresponds to "Apple ProRes 422 Proxy" in apples documents
> > > and now the decoder could output 12bit 444 without correcting the profile.
> > > IIUC this would be inconsistent
> > >
> > > This is not a major issue, its just metadata thats contradictionary
> > >
> > > Another minor issue is that this behavior is undocumented, or incorrectly
> > > documented
> > >
> > > For example for width and height we document in avcodec.h:
> > >  * - decoding: May be set by the user before opening the decoder if
> > > known e.g.
> > >  * from the container. Some decoders will require the
> > > dimensions
> > >  * to be set by the caller. During decoding, the de

Re: [FFmpeg-devel] [PATCH] libstr.c: Fixed rendezvous mode

2018-12-07 Thread Moritz Barsnick
On Fri, Dec 07, 2018 at 14:01:03 +, Adrian Grzeca wrote:

Please read the coding guidelines here:
https://www.ffmpeg.org/developer.html#Coding-Rules-1

Your indentation is incorrect, incl. use of tabs, which is not
supported in the ffmpeg code base. Also, your brackets and whitespace
are incorrect.

Furthermore, your commit message says "Fixed rendezvous mode", when
your change gives the impression that you're adding features:

> + { "localip", "IP address of network card to use in rendezvous mode 
> (Req. in rendezvous mode)", OFFSET(localip),   
> AV_OPT_TYPE_STRING,   { .str = NULL },  .flags = D|E },
> + { "localport", "Source port to use in rendezvous mode (Req. in 
> rendezvous mode)", OFFSET(localport),   AV_OPT_TYPE_STRING,   
> { .str = NULL },  .flags = D|E },

You need to explain in more detail what the actual issue is (is there a
trac ticket for this?), and how you are fixing it.

Furthermore:
> Subject: libstr.c: Fixed rendezvous mode

An ffmpeg commit message, or at least its header line, would read:
  avformat/libsrt: fix rendezvous mode

> + { "localip", "IP address of network card to use in rendezvous mode 
> (Req. in rendezvous mode)", OFFSET(localip),   
> AV_OPT_TYPE_STRING,   { .str = NULL },  .flags = D|E },

^
What does "Req." mean and why is it capitalized?

> + if(s->localip == NULL || s->localport == NULL) {

ffmpeg's style would be
if (!s->localip || !s->localport) {

Your code may have further issues which I cannot judge on.

Cheers,
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/h264_refs: reset MMCO when invalid mmco code is found

2018-12-07 Thread Paul B Mahol
On 12/7/18, Michael Niedermayer  wrote:
> On Fri, Dec 07, 2018 at 10:36:23AM +0100, Paul B Mahol wrote:
>> On 12/7/18, Paul B Mahol  wrote:
>> > On 12/7/18, Michael Niedermayer  wrote:
>> >> On Thu, Dec 06, 2018 at 03:26:41PM +0100, Paul B Mahol wrote:
>> >>> This recovers state with #7374 linked sample.
>> >>>
>> >>> Work funded by Open Broadcast Systems.
>> >>>
>> >>> Signed-off-by: Paul B Mahol 
>> >>> ---
>> >>>  libavcodec/h264_refs.c | 2 +-
>> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
>> >>> index eaf965e43d..5645a203a7 100644
>> >>> --- a/libavcodec/h264_refs.c
>> >>> +++ b/libavcodec/h264_refs.c
>> >>> @@ -718,6 +718,7 @@ int ff_h264_execute_ref_pic_marking(H264Context
>> >>> *h)
>> >>>  }
>> >>>  break;
>> >>>  case MMCO_RESET:
>> >>> +default:
>> >>>  while (h->short_ref_count) {
>> >>>  remove_short(h, h->short_ref[0]->frame_num, 0);
>> >>>  }
>> >>> @@ -730,7 +731,6 @@ int ff_h264_execute_ref_pic_marking(H264Context
>> >>> *h)
>> >>>  for (j = 0; j < MAX_DELAYED_PIC_COUNT; j++)
>> >>>  h->last_pocs[j] = INT_MIN;
>> >>>  break;
>> >>> -default: assert(0);
>> >>>  }
>> >>>  }
>> >>
>> >> mmco[i].opcode should not be invalid, its checked around the point
>> >> where
>> >> this
>> >> array is filled.
>> >> unless there is something iam missing
>> >
>> > Yes, you are missing big time.
>> > If you think by "checked" about those nice asserts they are not enabled
>> > at
>> > all.
>> >
>>
>> There is check for invalid opcode, but stored invalid opcode is not
>> changed.
>
> Theres no question that we end with a invalid value in the struct, but that
> is not intended to be in there. You can see that this is not intended by
> simply looking at the variable that holds the number of entries, it is
> not written at all in this case.
>
> So for example if this code stores 5 correct looking mmcos and the 6th is
> invalid, 6 are in the array but the number of entries is just left where it
> was, that could be maybe 3 or 8 or 1. Just "defaulting out" the invalid
> value
> later doesnt feel ideal.

Nope, mmco state is left in inconsistent state, and my patch fix it. As you
provided nothing valuable to consider as alternative I will apply it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2 v3] lavf/isom: add Dolby Vision sample entry codes for HEVC and H.264

2018-12-07 Thread Jan Ekström
On Wed, Dec 5, 2018 at 7:13 PM Jan Ekström  wrote:
>
> On Mon, Dec 3, 2018 at 3:19 AM Jan Ekström  wrote:
> >
> > From: Rodger Combs 
> >
> > These are registered identifiers at the MPEG-4 RA, which are
> > defined as to be utilized for Dolby Vision AVC/HEVC streams that
> > are not correctly presentable by standards-compliant AVC/HEVC players.
> >
> > According to the Dolby Vision specification for ISOBMFF, these sample
> > entry codes are specified to have the standard AVC or HEVC decoder
> > configuration box in addition to the Dolby custom DOVIConfigurationBox.
> > This is what enables us to decode the streams without custom parsing.
> >
> > For correct presentation information from the DOVIConfigurationBox
> > is required (YCbCr or modified ICtCP, SDR or HDR, base or enhancement
> > layer).
> > ---
>
> Gentle Ping?
>
> Jan

Ping^2?

And if nobody really cares, I will reverse the comments on the AVC
entries (since I seem to have gotten them the wrong way around when
adding the comments and commit message late at night) and apply the
set tomorrow morning. These additional identifiers and the comment
should not be affecting existing FATE samples.

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/mxfenc: calculate and store DAR from user SAR

2018-12-07 Thread Paul B Mahol
Fixes #5155

Signed-off-by: Paul B Mahol 
---
 libavformat/mxfenc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 3549b4137d..8f762c7eaf 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2726,6 +2726,14 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 }
 }
 
+if (st->codecpar->sample_aspect_ratio.num && 
st->codecpar->sample_aspect_ratio.den) {
+av_reduce(&sc->aspect_ratio.num, &sc->aspect_ratio.den,
+  st->codecpar->sample_aspect_ratio.num * st->codecpar->width,
+  st->codecpar->sample_aspect_ratio.den * 
st->codecpar->height, INT_MAX);
+} else if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+av_reduce(&sc->aspect_ratio.num, &sc->aspect_ratio.den, 
st->codecpar->width, st->codecpar->height, INT_MAX);
+}
+
 if (st->codecpar->codec_id == AV_CODEC_ID_MPEG2VIDEO) {
 if (!mxf_parse_mpeg2_frame(s, st, pkt, &ie)) {
 av_log(s, AV_LOG_ERROR, "could not get mpeg2 profile and level\n");
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/3] avcodec/avcodec: Document the data type for AV_PKT_DATA_MPEGTS_STREAM_ID

2018-12-07 Thread Michael Niedermayer
Signed-off-by: Michael Niedermayer 
---
 libavcodec/avcodec.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 3922e89331..fd7f60bf4a 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1328,7 +1328,7 @@ enum AVPacketSideDataType {
 AV_PKT_DATA_METADATA_UPDATE,
 
 /**
- * MPEGTS stream ID, this is required to pass the stream ID
+ * MPEGTS stream ID as uint8_t, this is required to pass the stream ID
  * information from the demuxer to the corresponding muxer.
  */
 AV_PKT_DATA_MPEGTS_STREAM_ID,
-- 
2.19.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/3] avformat/mpegts: Fix side data type for stream id

2018-12-07 Thread Michael Niedermayer
Signed-off-by: Michael Niedermayer 
---
 libavformat/mpegts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index edf6b5701d..a5e850e121 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -918,7 +918,7 @@ static void new_data_packet(const uint8_t *buffer, int len, 
AVPacket *pkt)
 
 static int new_pes_packet(PESContext *pes, AVPacket *pkt)
 {
-char *sd;
+uint8_t *sd;
 
 av_init_packet(pkt);
 
-- 
2.19.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 3/3] avformat/mov: Simplify get_stream_info_time()

2018-12-07 Thread Michael Niedermayer
Signed-off-by: Michael Niedermayer 
---
 libavformat/mov.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index ec57a05803..878e8a3ab2 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1234,16 +1234,12 @@ static int search_frag_moof_offset(MOVFragmentIndex 
*frag_index, int64_t offset)
 
 static int64_t get_stream_info_time(MOVFragmentStreamInfo * frag_stream_info)
 {
-
-if (frag_stream_info) {
-if (frag_stream_info->sidx_pts != AV_NOPTS_VALUE)
-return frag_stream_info->sidx_pts;
-if (frag_stream_info->first_tfra_pts != AV_NOPTS_VALUE)
-return frag_stream_info->first_tfra_pts;
-if (frag_stream_info->tfdt_dts != AV_NOPTS_VALUE)
-return frag_stream_info->tfdt_dts;
-}
-return AV_NOPTS_VALUE;
+av_assert0(frag_stream_info);
+if (frag_stream_info->sidx_pts != AV_NOPTS_VALUE)
+return frag_stream_info->sidx_pts;
+if (frag_stream_info->first_tfra_pts != AV_NOPTS_VALUE)
+return frag_stream_info->first_tfra_pts;
+return frag_stream_info->tfdt_dts;
 }
 
 static int64_t get_frag_time(MOVFragmentIndex *frag_index,
-- 
2.19.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libstr.c: Fixed rendezvous mode

2018-12-07 Thread Marton Balint



On Fri, 7 Dec 2018, Moritz Barsnick wrote:


On Fri, Dec 07, 2018 at 14:01:03 +, Adrian Grzeca wrote:

Please read the coding guidelines here:
https://www.ffmpeg.org/developer.html#Coding-Rules-1

Your indentation is incorrect, incl. use of tabs, which is not
supported in the ffmpeg code base. Also, your brackets and whitespace
are incorrect.

Furthermore, your commit message says "Fixed rendezvous mode", when
your change gives the impression that you're adding features:


+   { "localip", "IP address of network card to use in rendezvous mode (Req. 
in rendezvous mode)", OFFSET(localip),   AV_OPT_TYPE_STRING,   { .str = NULL 
},  .flags = D|E },


Please use option name "localaddr" instead because I believe UDP protocol 
has a similar option.



+   { "localport", "Source port to use in rendezvous mode (Req. in 
rendezvous mode)", OFFSET(localport),   AV_OPT_TYPE_STRING,   { .str = NULL }, 
 .flags = D|E },


AV_OPT_TYPE_INT



You need to explain in more detail what the actual issue is (is there a
trac ticket for this?), and how you are fixing it.

Furthermore:

Subject: libstr.c: Fixed rendezvous mode


An ffmpeg commit message, or at least its header line, would read:
 avformat/libsrt: fix rendezvous mode


+   { "localip", "IP address of network card to use in rendezvous mode (Req. 
in rendezvous mode)", OFFSET(localip),   AV_OPT_TYPE_STRING,   { .str = NULL 
},  .flags = D|E },

   ^
What does "Req." mean and why is it capitalized?


I'd just drop the extra info parenthesis, the text before that makes it 
clear that these options are used in rendezvous mode only.





+   if(s->localip == NULL || s->localport == NULL) {


ffmpeg's style would be
   if (!s->localip || !s->localport) {

Your code may have further issues which I cannot judge on.


doc/protocols.texi update is missing.

Thanks,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libavcodec/zmbvenc.c: don't allow motion estimation out of range.

2018-12-07 Thread Matthew Fearnley
Hi Tomas, thanks for looking through my patch.

> > Practically, this patch fixes graphical glitches e.g. when reencoding
the
> > Commander Keen sample video with me_range 65 or higher:
> >
> > ffmpeg -i keen4e_000.avi -c:v zmbv -me_range 65 keen4e_me65.avi

> I'd expect this problem to pop up with -me_range 64 too, no?

I initially thought this would be the case, but taking the tx loop and
removing the edge constraints:

for(tx = x - c->range; tx < x + c->range; tx++){
...
dx = tx - x;

The effective range is (-c->range) <= dx < (c->range), meaning when
c->range = me_range = 64, the dx value ranges from -64 to 63, which happens
to be exactly in bounds.
So I could have just capped me_range to 64, and that would have fixed the
bug...

But more generally, I've concluded the '<' sign is a mistake, not just
because of the general asymmetry, but because of the way it prevents tx,ty
reaching the bottom/right edges.
In practice it means, for example, that if the screen pans left to right,
the bottom edge will have to match against blocks elsewhere in the image.

> I went over the patch, and it looks fine. But what's up with the xored
> logic? It seems like it would compute xored always from the bottom-
> right-most MV. The loop in zmbv_me() should probably have a temporary
> xored and only output to *xored in if(tv < bv)..

Hmm, you're right.  In most cases, the code actually works anyway - because
when *xored==0, the block entropy returned by block_cmp() is supposed to be
0 anyway, so it still finishes then.
But... I've realised there are some exceptions to this:
- the entropy calculations in block_cmp() use a lookup table
(score_tab[256]), which assumes each block has 16*16 pixels.  This will
only be valid when the dimensions are a multiple of 16, otherwise the
bottom/right edge blocks may be smaller.
- I've just realised the lookup table only goes up to 255.  If all 16*16
xored pixels are the same value, it will hit the 256th entry!  (I'm
surprised valgrind hasn't found this..?)

All that said, *xored==0 is actually the most desirable outcome because it
means the block doesn't need to be output.
So if *xored is 0, the best thing to do is probably to finish immediately
(making sure *mx,*my are set), without processing any more MVs.
And then it doesn't matter (from a correctness point of view, at least) if
block_cmp() gives bad entropy results, as long as *xored is set correctly.

Note: the code currently exits on bv==0.  It's a very good result, but it
does not always imply the most desirable case, because it will happen if
the xored block contains all 42's (etc), not just all 0's.
It's obviously highly compressible, but it would still be better if the
block could be omitted entirely.  Maybe it's an acceptable time/space
tradeoff though.  I don't know...

So I guess there are at least two more fixes that need to be made:
- score_tab[] should have 257 elements (well actually, it should have
ZMBV_BLOCK*ZMBV_BLOCK+1 elements.)
- zmbv_me() should set *mx,*my and finish early, if block_cmp() (either at
the start or in the loop) sets *xored to 0.

Matthew
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/jpeg2000dec: Skip DWT if nothing is coded

2018-12-07 Thread Michael Niedermayer
On Sun, Dec 02, 2018 at 03:22:28AM +0100, Michael Niedermayer wrote:
> Improves speed in uncommon case
> 
> Fixes: Timeout
> Fixes: 
> 10964/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5132066034286592
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/jpeg2000dec.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)

will apply this with a small bugfix in it


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk



signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/dxv: Check that there is enough data to decompress

2018-12-07 Thread Michael Niedermayer
On Sat, Dec 01, 2018 at 10:16:20PM +0100, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 
> 10979/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXV_fuzzer-6178582203203584
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/dxv.c | 6 ++
>  1 file changed, 6 insertions(+)

will apply

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc : Added an option to ignore IO errors

2018-12-07 Thread Steven Liu


> 在 2018年12月7日,下午12:49,Karthick J  写道:
> 
> Useful for long duration runs with network output
> ---
> doc/muxers.texi  |  3 +++
> libavformat/hlsenc.c | 41 +++--
> 2 files changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index ca10741900..8eefcf1e82 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1018,6 +1018,9 @@ Use persistent HTTP connections. Applicable only for 
> HTTP output.
> @item timeout
> Set timeout for socket I/O operations. Applicable only for HTTP output.
> 
> +@item -ignore_io_errors
> +Ignore IO errors during open, write and delete. Useful for long-duration 
> runs with network output.
> +
> @end table
> 
> @anchor{ico}
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 42adcfbab1..bdd2a113bd 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -227,6 +227,7 @@ typedef struct HLSContext {
> AVIOContext *m3u8_out;
> AVIOContext *sub_m3u8_out;
> int64_t timeout;
> +int ignore_io_errors;
> } HLSContext;
> 
> static int hlsenc_io_open(AVFormatContext *s, AVIOContext **pb, char 
> *filename,
> @@ -496,8 +497,11 @@ static int hls_delete_old_segments(AVFormatContext *s, 
> HLSContext *hls,
> proto = avio_find_protocol_name(s->url);
> if (hls->method || (proto && !av_strcasecmp(proto, "http"))) {
> av_dict_set(&options, "method", "DELETE", 0);
> -if ((ret = vs->avf->io_open(vs->avf, &out, path, 
> AVIO_FLAG_WRITE, &options)) < 0)
> +if ((ret = vs->avf->io_open(vs->avf, &out, path, 
> AVIO_FLAG_WRITE, &options)) < 0) {
> +if (hls->ignore_io_errors)
> +ret = 0;
> goto fail;
> +}
> ff_format_io_close(vs->avf, &out);
> } else if (unlink(path) < 0) {
> av_log(hls, AV_LOG_ERROR, "failed to delete old segment %s: %s\n",
> @@ -525,6 +529,8 @@ static int hls_delete_old_segments(AVFormatContext *s, 
> HLSContext *hls,
> if (hls->method || (proto && !av_strcasecmp(proto, "http"))) {
> av_dict_set(&options, "method", "DELETE", 0);
> if ((ret = vs->vtt_avf->io_open(vs->vtt_avf, &out, sub_path, 
> AVIO_FLAG_WRITE, &options)) < 0) {
> +if (hls->ignore_io_errors)
> +ret = 0;
> av_free(sub_path);
> goto fail;
> }
> @@ -1380,8 +1386,11 @@ static int hls_window(AVFormatContext *s, int last, 
> VariantStream *vs)
> 
> set_http_options(s, &options, hls);
> snprintf(temp_filename, sizeof(temp_filename), use_temp_file ? "%s.tmp" : 
> "%s", vs->m3u8_name);
> -if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 
> 0)
> +if ((ret = hlsenc_io_open(s, &hls->m3u8_out, temp_filename, &options)) < 
> 0) {
> +if (hls->ignore_io_errors)
> +ret = 0;
> goto fail;
> +}
> 
> for (en = vs->segments; en; en = en->next) {
> if (target_duration <= en->duration)
> @@ -1428,8 +1437,11 @@ static int hls_window(AVFormatContext *s, int last, 
> VariantStream *vs)
> ff_hls_write_end_list(hls->m3u8_out);
> 
> if( vs->vtt_m3u8_name ) {
> -if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, 
> &options)) < 0)
> +if ((ret = hlsenc_io_open(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name, 
> &options)) < 0) {
> +if (hls->ignore_io_errors)
> +ret = 0;
> goto fail;
> +}
> ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, 
> hls->allowcache,
>  target_duration, sequence, 
> PLAYLIST_TYPE_NONE);
> for (en = vs->segments; en; en = en->next) {
> @@ -1452,7 +1464,6 @@ fail:
> hlsenc_io_close(s, &hls->sub_m3u8_out, vs->vtt_m3u8_name);
> if (use_temp_file)
> ff_rename(temp_filename, vs->m3u8_name, s);
> -
> if (ret >= 0 && hls->master_pl_name)
> if (create_master_playlist(s, vs) < 0)
> av_log(s, AV_LOG_WARNING, "Master playlist creation failed\n");
> @@ -1611,13 +1622,19 @@ static int hls_start(AVFormatContext *s, 
> VariantStream *vs)
> if (err < 0)
> return err;
> } else if (c->segment_type != SEGMENT_TYPE_FMP4) {
> -if ((err = hlsenc_io_open(s, &oc->pb, oc->url, &options)) < 0)
> +if ((err = hlsenc_io_open(s, &oc->pb, oc->url, &options)) < 0) {
> +if (c->ignore_io_errors)
> +err = 0;
> goto fail;
> +}
> }
> if (vs->vtt_basename) {
> set_http_options(s, &options, c);
> -if ((err = hlsenc_io_open(s, &vtt_oc->pb, vtt_oc->url, &options)) < 
> 0)
> +if ((err = hlsenc_io_open(s, &vtt_oc->pb, vtt_oc->url, &options)) < 
> 0) {
> +if (c->ignore_io_errors)
> +err = 0;
> goto fail;
> +}
> }
>