Re: [FFmpeg-devel] [PATCH 1/2] avcodec/vaapi: slice_vertical_position starts from zero for the second field

[Xiang, Haihao]

On Wed, 2018-05-09 at 14:00 +0200, Jerome Borsboom wrote:
> > > Contrary to VC-1 spec, VAAPI expects the row address of the first
> > > macroblock row in the first slice to start from zero for the second
> > > field in a field interlaced picture.
> > > 
> > > Signed-off-by: Jerome Borsboom 
> > > ---
> > > This patch set adds support for hardware decoding multi-slice field
> > > interlaced
> > > pictures. With this patch set, the SA10180 test file decodes correctly
> > > with
> > > VAAPI hardware acceleration. This was succesfully tested on Intel Haswell
> > > platform.
> > > 
> > 
> > I still see lots of artifacts for a multi-slice field interfaced VC-1 video
> > on
> > Coffee Lake, maybe we should fix it in the driver 
> > 
> > Thanks
> > Haihao
> 
> I suppose you also applied the second part of this patch and still see
> artifacts. I cannot check for Coffee Lake, but there may be issues with
> the VAAPI driver for CL platform. The patches are just a copy of the
> multi-slice support for frame interlaced images, so nothing special there.
> 
> Could you share (part of) the video you used to check on Coffee Lake so
> that I can see how Haswell performs?
> 

I apologize that I used a video with Luma/Chroma scaling set which is not
supported by the driver. I confirmed your patchset works for me with other
multi-slice field interfaced VC-1 video.

Thanks
Haihao


> 
> Regards,
> Jerome
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavd/v4l2: Add ARGB and XRGB packed pixel formats

[Антон Леонтьев]

> 2018-05-08 15:11 GMT+02:00, Anton Leontiev :
>> Formats ARGB32, XRGB32, ABGR32, and XBGR32 were added to
>> V4L2 instead of ill-defined deprecated RGB32/BGR32 pixel formats.
>
> Just curious because I am quite sure I tested them:
> Why / how are they ill-defined?

According to [1-2]:

  The meaning of their alpha bits (a) are ill-defined and interpreted as in
  either the corresponding ARGB or XRGB format, depending on the driver.

[1] https://git.kernel.org/torvalds/c/977ff0e4fb3460d
[2] https://www.kernel.org/doc/html/latest/media/uapi/v4l/pixfmt-packed-rgb.html

>> diff --git a/libavdevice/v4l2-common.c b/libavdevice/v4l2-common.c
>> index 196c09b7fc..d48ae2efa1 100644
>> --- a/libavdevice/v4l2-common.c
>> +++ b/libavdevice/v4l2-common.c
>> @@ -34,6 +34,10 @@ const struct fmt_map ff_fmt_conversion_table[] = {
>>  { AV_PIX_FMT_RGB565BE,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_RGB565X },
>>  { AV_PIX_FMT_BGR24,   AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_BGR24   },
>>  { AV_PIX_FMT_RGB24,   AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_RGB24   },
>> +{ AV_PIX_FMT_BGR0,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_XBGR32  },
>> +{ AV_PIX_FMT_0RGB,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_XRGB32  },
>> +{ AV_PIX_FMT_BGRA,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_ABGR32  },
>> +{ AV_PIX_FMT_ARGB,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_ARGB32  },
>
> Needs an "#ifdef" guard like below.

Will add in the next version. Thank you for review.

Best regards,

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

Re: [FFmpeg-devel] avformat/mpegts: set AV_DISPOSITION_DESCRIPTIONS for OIPF cases

[Łukasz Krzciuk]

Lets skip a testcase. It will be not easy to me to prepare it.

Regards,

*Łukasz Krzciuk*
Developer

Vewd
ul. Grabarska 2, Pegaz 2A, 50-079 Wrocław, Polska


On Thu, May 10, 2018 at 3:09 AM, Michael Niedermayer  wrote:

> On Wed, May 09, 2018 at 10:19:26AM +0200, Łukasz Krzciuk wrote:
> > Any updates on this issue?
>
> about adding a testcase ?
> adding a huge sample file is not possible.
> It either has to be made smaller or generated by a muxer
> or last resort, no test
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The worst form of inequality is to try to make unequal things equal.
> -- Aristotle
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/nvdec: avoid needless copy of output frame

[Timo Rothenpieler]

Am 10.05.2018 um 04:51 schrieb Wang Bin:




-offset += cpy.Height;
+frame->buf[1] = av_buffer_create((uint8_t *)unmap_data,
sizeof(*unmap_data),
+ nvdec_unmap_mapped_frame,
(void*)devptr,
+ AV_BUFFER_FLAG_READONLY);
+if (!frame->buf[1]) {
+ret = AVERROR(ENOMEM);
+goto copy_fail;
  }



If AVFrame.buf[i] is non-NULL, then buf[j] must be non-NULL for all j < i,
see libavutil/frame.h. So either change the comment in frame.h or change
your implementation is required


buf[0] is filled by the hw_frames_ctx, with a dummy buffer, but it is 
filled.

So after this buf[0] and buf[1] are filled, so I don't see the problem?



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/nvdec_hevc: Fix scaling lists

[Timo Rothenpieler]

Am 10.05.2018 um 03:48 schrieb Philip Langdale:

The main issue here was the use of [i] instead of [i * 3] for the 32x32
matrix. As part of fixing this, I changed the code to match that used
in vdpau_hevc, which I spent a lot of time verifying.

I also changed to calculating NumPocTotalCurr using the existing helper,
which is what vdpau does.
---
  libavcodec/nvdec_hevc.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/nvdec_hevc.c b/libavcodec/nvdec_hevc.c
index 008963130b..e04a701f3a 100644
--- a/libavcodec/nvdec_hevc.c
+++ b/libavcodec/nvdec_hevc.c
@@ -58,12 +58,13 @@ static void fill_scaling_lists(CUVIDHEVCPICPARAMS *ppc, 
const HEVCContext *s)
  ppc->ScalingList16x16[i][j] = sl->sl[2][i][pos];
  
  if (i < 2)

-ppc->ScalingList32x32[i][j] = sl->sl[3][i][pos];
+ppc->ScalingList32x32[i][j] = sl->sl[3][i * 3][pos];
  }
-}
  
-memcpy(ppc->ScalingListDCCoeff16x16, sl->sl_dc[0], sizeof(ppc->ScalingListDCCoeff16x16));

-memcpy(ppc->ScalingListDCCoeff32x32, sl->sl_dc[1], 
sizeof(ppc->ScalingListDCCoeff32x32));
+ppc->ScalingListDCCoeff16x16[i] = sl->sl_dc[0][i];
+if (i < 2)
+ppc->ScalingListDCCoeff32x32[i] = sl->sl_dc[1][i * 3];
+}
  }
  
  static int nvdec_hevc_start_frame(AVCodecContext *avctx,

@@ -166,8 +167,7 @@ static int nvdec_hevc_start_frame(AVCodecContext *avctx,
  
  .NumBitsForShortTermRPSInSlice= s->sh.short_term_rps ? s->sh.short_term_ref_pic_set_size : 0,

  .NumDeltaPocsOfRefRpsIdx  = s->sh.short_term_rps ? 
s->sh.short_term_rps->rps_idx_num_delta_pocs : 0,
-.NumPocTotalCurr  = 
s->rps[ST_CURR_BEF].nb_refs + s->rps[ST_CURR_AFT].nb_refs +
-
s->rps[LT_CURR].nb_refs,
+.NumPocTotalCurr  = 
ff_hevc_frame_nb_refs(s),


This produces a warning because ff_hevc_frame_nb_refs discards the const 
from s.
I don't really see a reason why it would need to do that, so I'd say the 
apropiate fix is to make ff_hevc_frame_nb_refs take const instead.




smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec/hevcdec: make ff_hevc_frame_nb_refs take a const pointer

[Timo Rothenpieler]

---
 libavcodec/hevc_refs.c | 4 ++--
 libavcodec/hevcdec.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
index 68c730edcc..7cf3a55725 100644
--- a/libavcodec/hevc_refs.c
+++ b/libavcodec/hevc_refs.c
@@ -508,12 +508,12 @@ fail:
 return ret;
 }
 
-int ff_hevc_frame_nb_refs(HEVCContext *s)
+int ff_hevc_frame_nb_refs(const HEVCContext *s)
 {
 int ret = 0;
 int i;
 const ShortTermRPS *rps = s->sh.short_term_rps;
-LongTermRPS *long_rps   = &s->sh.long_term_rps;
+const LongTermRPS *long_rps = &s->sh.long_term_rps;
 
 if (rps) {
 for (i = 0; i < rps->num_negative_pics; i++)
diff --git a/libavcodec/hevcdec.h b/libavcodec/hevcdec.h
index b311edc8ae..f0f588f2b8 100644
--- a/libavcodec/hevcdec.h
+++ b/libavcodec/hevcdec.h
@@ -544,7 +544,7 @@ int ff_hevc_res_scale_sign_flag(HEVCContext *s, int idx);
 /**
  * Get the number of candidate references for the current frame.
  */
-int ff_hevc_frame_nb_refs(HEVCContext *s);
+int ff_hevc_frame_nb_refs(const HEVCContext *s);
 
 int ff_hevc_set_new_ref(HEVCContext *s, AVFrame **frame, int poc);
 
-- 
2.17.0

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

Re: [FFmpeg-devel] [PATCH] avcodec/hevcdec: make ff_hevc_frame_nb_refs take a const pointer

[Philip Langdale]

On Thu, 10 May 2018 12:23:01 +0200
Timo Rothenpieler  wrote:

> ---
>  libavcodec/hevc_refs.c | 4 ++--
>  libavcodec/hevcdec.h   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
> index 68c730edcc..7cf3a55725 100644
> --- a/libavcodec/hevc_refs.c
> +++ b/libavcodec/hevc_refs.c
> @@ -508,12 +508,12 @@ fail:
>  return ret;
>  }
>  
> -int ff_hevc_frame_nb_refs(HEVCContext *s)
> +int ff_hevc_frame_nb_refs(const HEVCContext *s)
>  {
>  int ret = 0;
>  int i;
>  const ShortTermRPS *rps = s->sh.short_term_rps;
> -LongTermRPS *long_rps   = &s->sh.long_term_rps;
> +const LongTermRPS *long_rps = &s->sh.long_term_rps;
>  
>  if (rps) {
>  for (i = 0; i < rps->num_negative_pics; i++)
> diff --git a/libavcodec/hevcdec.h b/libavcodec/hevcdec.h
> index b311edc8ae..f0f588f2b8 100644
> --- a/libavcodec/hevcdec.h
> +++ b/libavcodec/hevcdec.h
> @@ -544,7 +544,7 @@ int ff_hevc_res_scale_sign_flag(HEVCContext *s,
> int idx); /**
>   * Get the number of candidate references for the current frame.
>   */
> -int ff_hevc_frame_nb_refs(HEVCContext *s);
> +int ff_hevc_frame_nb_refs(const HEVCContext *s);
>  
>  int ff_hevc_set_new_ref(HEVCContext *s, AVFrame **frame, int poc);
>  

Looks good. Thanks for doing this.

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

Re: [FFmpeg-devel] [PATCH] avcodec/nvdec_hevc: Fix scaling lists

[Philip Langdale]

On Thu, 10 May 2018 12:18:13 +0200
Timo Rothenpieler  wrote:

> Am 10.05.2018 um 03:48 schrieb Philip Langdale:
> > The main issue here was the use of [i] instead of [i * 3] for the
> > 32x32 matrix. As part of fixing this, I changed the code to match
> > that used in vdpau_hevc, which I spent a lot of time verifying.
> > 
> > I also changed to calculating NumPocTotalCurr using the existing
> > helper, which is what vdpau does.
> > ---
> >   libavcodec/nvdec_hevc.c | 12 ++--
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libavcodec/nvdec_hevc.c b/libavcodec/nvdec_hevc.c
> > index 008963130b..e04a701f3a 100644
> > --- a/libavcodec/nvdec_hevc.c
> > +++ b/libavcodec/nvdec_hevc.c
> > @@ -58,12 +58,13 @@ static void
> > fill_scaling_lists(CUVIDHEVCPICPARAMS *ppc, const HEVCContext *s)
> > ppc->ScalingList16x16[i][j] = sl->sl[2][i][pos]; 
> >   if (i < 2)
> > -ppc->ScalingList32x32[i][j] = sl->sl[3][i][pos];
> > +ppc->ScalingList32x32[i][j] = sl->sl[3][i *
> > 3][pos]; }
> > -}
> >   
> > -memcpy(ppc->ScalingListDCCoeff16x16, sl->sl_dc[0],
> > sizeof(ppc->ScalingListDCCoeff16x16));
> > -memcpy(ppc->ScalingListDCCoeff32x32, sl->sl_dc[1],
> > sizeof(ppc->ScalingListDCCoeff32x32));
> > +ppc->ScalingListDCCoeff16x16[i] = sl->sl_dc[0][i];
> > +if (i < 2)
> > +ppc->ScalingListDCCoeff32x32[i] = sl->sl_dc[1][i * 3];
> > +}
> >   }
> >   
> >   static int nvdec_hevc_start_frame(AVCodecContext *avctx,
> > @@ -166,8 +167,7 @@ static int
> > nvdec_hevc_start_frame(AVCodecContext *avctx, 
> >   .NumBitsForShortTermRPSInSlice=
> > s->sh.short_term_rps ? s->sh.short_term_ref_pic_set_size :
> > 0, .NumDeltaPocsOfRefRpsIdx  =
> > s->sh.short_term_rps ?
> > s->sh.short_term_rps->rps_idx_num_delta_pocs : 0,
> > -.NumPocTotalCurr  =
> > s->rps[ST_CURR_BEF].nb_refs + s->rps[ST_CURR_AFT].nb_refs +
> > -
> > s->rps[LT_CURR].nb_refs,
> > +.NumPocTotalCurr  =
> > ff_hevc_frame_nb_refs(s),  
> 
> This produces a warning because ff_hevc_frame_nb_refs discards the
> const from s.
> I don't really see a reason why it would need to do that, so I'd say
> the apropiate fix is to make ff_hevc_frame_nb_refs take const instead.
> 

Yes - I noticed this but then forgot to address it. I'll push after you
push your change to make it const.

Thanks.

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

Re: [FFmpeg-devel] [PATCH 2/3] libavcodec: v4l2m2m: output AVDRMFrameDescriptor

[wm4]

On Thu, 10 May 2018 01:24:42 +0200
Jorge Ramirez-Ortiz  wrote:

> On 05/09/2018 04:11 PM, Mark Thompson wrote:
> > On 09/05/18 08:57, Jorge Ramirez-Ortiz wrote:  
> >> On 05/09/2018 01:33 AM, Mark Thompson wrote:  
> >>> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> >>> index ed5193ecc1..2b33badb08 100644
> >>> --- a/libavcodec/v4l2_m2m_dec.c
> >>> +++ b/libavcodec/v4l2_m2m_dec.c
> >>> @@ -23,12 +23,18 @@
> >>>      #include 
> >>>    #include 
> >>> +
> >>> +#include "libavutil/hwcontext.h"
> >>> +#include "libavutil/hwcontext_drm.h"
> >>>    #include "libavutil/pixfmt.h"
> >>>    #include "libavutil/pixdesc.h"
> >>>    #include "libavutil/opt.h"
> >>>    #include "libavcodec/avcodec.h"
> >>>    #include "libavcodec/decode.h"
> >>>    +#include "libavcodec/hwaccel.h"
> >>> +#include "libavcodec/internal.h"
> >>> +
> >>>    #include "v4l2_context.h"
> >>>    #include "v4l2_m2m.h"
> >>>    #include "v4l2_fmt.h"
> >>> @@ -183,6 +189,15 @@ static av_cold int v4l2_decode_init(AVCodecContext 
> >>> *avctx)
> >>>    capture->av_codec_id = AV_CODEC_ID_RAWVIDEO;
> >>>    capture->av_pix_fmt = avctx->pix_fmt;
> >>>    +    /* the client requests the codec to generate DRM frames:
> >>> + *   - data[0] will therefore point to the returned 
> >>> AVDRMFrameDescriptor
> >>> + *   check the ff_v4l2_buffer_to_avframe conversion function.
> >>> + *   - the DRM frame format is passed in the DRM frame descriptor 
> >>> layer.
> >>> + *   check the v4l2_get_drm_frame function.
> >>> + */
> >>> +    if (avctx->pix_fmt == AV_PIX_FMT_DRM_PRIME)
> >>> +    s->output_drm = 1;
> >>> +
> >>>    ret = ff_v4l2_m2m_codec_init(avctx);
> >>>    if (ret) {
> >>>    av_log(avctx, AV_LOG_ERROR, "can't configure decoder\n");
> >>> @@ -202,6 +217,11 @@ static const AVOption options[] = {
> >>>    { NULL},
> >>>    };  
> >> As a follow up to your comment on pixel format negotiation 
> >> (AvCodecContext.getformat), notice that this is a tentative request from 
> >> the user to select a pixel format.
> >> The actual pixel format negotiation - where the decoder will select the 
> >> pixel format- will happen later during v4l2_try_start.  
> > Indeed.  get_format() will have to be called during the pixel format 
> > negotiation so that the user can pick between whatever the supported 
> > software format is (NV12, NV21, YUV420P P010, whatever) or the DRM-PRIME 
> > object hardware format (if supported).  
> 
> but in the use case that we are trying to implement, it is the user - 
> not the codec - the one that has to specifically request DRM support to 
> libavcodec since the v4l2m2m decoder can provide either of them 
> indistinctly.
> 
> it is just not clear to me how the libavcodec client can request the DRM 
> pix_fmt if not tentatively setting the pix_fmt during v4l2_decode_init.
> 
> 
> >
> > AVCodecContext.pix_fmt is intended to be set by the decoder to say what 
> > pix_fmt it intends to produce (though even in that role it's highly dubious 
> > given threaded decoders and stream changes).  For historical reasons it is 
> > also allowed to be set externally (because of libavformat interactions), 
> > but this shouldn't be used for configuration.
> >  
> >> This change enables the v4l2m2m decoder to output either dmabuf 
> >> descriptors to be consumed by a DRM or video frame formats to be consumed 
> >> by SDL (for instance).
> >> As an example, these changes have been tested with ffplay (SDL based 
> >> display) and a simple DRM application [1]
> >>
> >> Lukas tested with other tools.
> >>
> >> [1]https://github.com/baylibre/ffmpeg-drm  
> > We should make this usable in the ffmpeg application too.  The DRM object 
> > format is works fine in ffmpeg already with Rockchip decoder (consumed by 
> > the hwmap/hwdownload filters, or by mapping to OpenCL), but that doesn't 
> > need the format selection part.  (There are also kmsgrab and VAAPI, but 
> > those aren't making DRM PRIME frames directly at a decoder.)  I believe 
> > this should be straightforward with a small modification to get_format() in 
> > ffmpeg.c to accept AV_CODEC_HW_CONFIG_METHOD_INTERNAL, I can look at this 
> > once we have a codec which will need it.  
> 
> so what would be the sequence of calls for an libavcodec client to 
> request the DRM format?
> https://github.com/BayLibre/ffmpeg-drm/blob/master/main.c#L598

Like in 100% of all existing cases in ffmpeg: set a get_format
callback, and return the format from it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/bluray: translate a read of 0 to EOF

[Jan Ekström]

On Sun, May 6, 2018 at 11:16 PM, Michael Niedermayer
 wrote:
> On Sun, May 06, 2018 at 06:55:01PM +0300, Jan Ekström wrote:
>> Thanks, pushed. This should be back-ported to release/4.0 I guess?
>
> probably yes
>
> thanks
>

Finally got the time, cherry-pick -x'd and pushed to release/4.0.

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

[FFmpeg-devel] [PATCH v2] lavd/v4l2: Add ARGB and XRGB packed pixel formats

[Anton Leontiev]

Formats ARGB32, XRGB32, ABGR32, and XBGR32 were added to V4L2 instead
of ill-defined deprecated RGB32/BGR32 pixel formats.

When pixel format is not specified explicitly FFmpeg tries formats in
order in which they are stored in the table. Therefore formats are
sorted as follows: BGR is preferred over RGB and XBGR is preferred
over ARGB, because it could give better performance by ignoring alpha
component.
---
 libavdevice/v4l2-common.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavdevice/v4l2-common.c b/libavdevice/v4l2-common.c
index 196c09b7fc..2d6bfac8ca 100644
--- a/libavdevice/v4l2-common.c
+++ b/libavdevice/v4l2-common.c
@@ -34,6 +34,12 @@ const struct fmt_map ff_fmt_conversion_table[] = {
 { AV_PIX_FMT_RGB565BE,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_RGB565X },
 { AV_PIX_FMT_BGR24,   AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_BGR24   },
 { AV_PIX_FMT_RGB24,   AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_RGB24   },
+#ifdef V4L2_PIX_FMT_XBGR32
+{ AV_PIX_FMT_BGR0,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_XBGR32  },
+{ AV_PIX_FMT_0RGB,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_XRGB32  },
+{ AV_PIX_FMT_BGRA,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_ABGR32  },
+{ AV_PIX_FMT_ARGB,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_ARGB32  },
+#endif
 { AV_PIX_FMT_BGR0,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_BGR32   },
 { AV_PIX_FMT_0RGB,AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_RGB32   },
 { AV_PIX_FMT_GRAY8,   AV_CODEC_ID_RAWVIDEO, V4L2_PIX_FMT_GREY},
-- 
2.17.0

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

[FFmpeg-devel] [GSoC] Qualification task (?) FFserver, further implementation

[Stephan Holljes]

This patchset contains a version that is more flexible than the previous
iteration.

To become independent of the specific http server implementation used I
added an interface that http server implementations have to provide. As
mentioned for the time being this is still the libavformat http server,
but I think it should be rather trivial to extend it to support more
different implementations.

I hope I have not missed functions that servers must provide and that
the interface I chose is exhaustive enough to provide all functionality
needed.

As always, comments welcome :)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/5] ffserver: Implement refcounted segments.

[Stephan Holljes]

Signed-off-by: Stephan Holljes 
---
 segment.c | 167 ++
 segment.h | 114 ++
 2 files changed, 281 insertions(+)
 create mode 100644 segment.c
 create mode 100644 segment.h

diff --git a/segment.c b/segment.c
new file mode 100644
index 000..afb6d80
--- /dev/null
+++ b/segment.c
@@ -0,0 +1,167 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include "segment.h"
+#include 
+
+#include 
+#include 
+
+
+void segment_save(struct Segment *seg, const char *filename)
+{
+AVFormatContext *ofmt_ctx = NULL;
+int ret;
+
+avformat_alloc_output_context2(&ofmt_ctx, NULL, NULL, filename);
+if (!ofmt_ctx) {
+av_log(NULL, AV_LOG_ERROR, "Could not allocate output to save Segment 
%d.\n", seg->id);
+return;
+}
+
+if ((ret = avio_open(&ofmt_ctx->pb, filename, AVIO_FLAG_WRITE)) < 0) {
+av_log(ofmt_ctx, AV_LOG_ERROR,
+"Could not open output io context to save Segment %d: %s.\n", 
seg->id, av_err2str(ret));
+return;
+}
+
+avio_write(ofmt_ctx->pb, seg->buf, seg->size);
+avio_flush(ofmt_ctx->pb);
+avio_close(ofmt_ctx->pb);
+avformat_free_context(ofmt_ctx);
+}
+
+void segment_free(struct Segment *seg)
+{
+av_log(NULL, AV_LOG_DEBUG, "Freeing segment\n");
+avformat_free_context(seg->fmt_ctx);
+av_free(seg->io_ctx->buffer);
+av_free(seg->io_ctx);
+av_free(seg->buf);
+av_free(seg->ts);
+av_free(seg);
+}
+
+void segment_ref(struct Segment *seg)
+{
+pthread_mutex_lock(&seg->nb_read_lock);
+seg->nb_read++;
+av_log(NULL, AV_LOG_DEBUG, "%04d  ref Readers: %d\n", seg->id, 
seg->nb_read);
+pthread_mutex_unlock(&seg->nb_read_lock);
+}
+
+void segment_unref(struct Segment *seg)
+{
+pthread_mutex_lock(&seg->nb_read_lock);
+seg->nb_read--;
+pthread_mutex_unlock(&seg->nb_read_lock);
+av_log(NULL, AV_LOG_DEBUG, "%04d unref Readers: %d\n", seg->id, 
seg->nb_read);
+if (seg->nb_read == 0) {
+segment_free(seg);
+}
+}
+
+int segment_write(void *opaque, unsigned char *buf, int buf_size)
+{
+struct Segment *seg = (struct Segment*) opaque;
+seg->size += buf_size;
+seg->buf = (unsigned char*) av_realloc(seg->buf, seg->size);
+memcpy(seg->buf + seg->size - buf_size, buf, buf_size);
+return buf_size;
+}
+
+int segment_read(void *opaque, unsigned char *buf, int buf_size)
+{
+struct SegmentReadInfo *info = (struct SegmentReadInfo*) opaque;
+buf_size = buf_size < info->left ? buf_size : info->left;
+
+/* copy internal buffer data to buf */
+memcpy(buf, info->buf, buf_size);
+info->buf  += buf_size;
+info->left -= buf_size;
+return buf_size ? buf_size : AVERROR_EOF;
+}
+
+
+void segment_close(struct Segment *seg)
+{
+av_write_trailer(seg->fmt_ctx);
+}
+
+void segment_init(struct Segment **seg_p, AVFormatContext *fmt)
+{
+int ret;
+int i;
+AVStream *in_stream, *out_stream;
+struct Segment *seg = (struct Segment*) av_malloc(sizeof(struct Segment));
+
+seg->ifmt = fmt->iformat;
+seg->fmt_ctx = NULL;
+seg->nb_read = 0;
+seg->size = 0;
+seg->ts = NULL;
+seg->ts_len = 0;
+seg->buf = NULL;
+seg->avio_buffer = (unsigned char*) av_malloc(AV_BUFSIZE);
+pthread_mutex_init(&seg->nb_read_lock, NULL);
+seg->io_ctx = avio_alloc_context(seg->avio_buffer, AV_BUFSIZE, 1, seg, 
NULL, &segment_write, NULL);
+seg->io_ctx->seekable = 0;
+avformat_alloc_output_context2(&seg->fmt_ctx, NULL, "matroska", NULL);
+if ((ret = av_opt_set_int(seg->fmt_ctx, "flush_packets", 1, 
AV_OPT_SEARCH_CHILDREN)) < 0) {
+av_log(seg->fmt_ctx, AV_LOG_WARNING, "Could not set flush_packets!\n");
+}
+
+seg->fmt_ctx->flags |= AVFMT_FLAG_GENPTS;
+seg->fmt_ctx->oformat->flags &= AVFMT_NOFILE;
+
+av_log(seg->fmt_ctx, AV_LOG_DEBUG, "Initializing segment\n");
+
+for (i = 0; i < fmt->nb_streams; i++) {
+in_stream = fmt->streams[i];
+out_stream = avformat_new_stream(seg->fmt_ctx, NULL);
+if (!out_stream) {
+av_log(seg->fmt_ctx, AV_LOG_WARNING, "Failed allocating output 
stream\n");
+continue;
+}
+ret =

[FFmpeg-devel] [PATCH 2/5] ffserver: Implement publisher

[Stephan Holljes]

Signed-off-by: Stephan Holljes 
---
 publisher.c | 306 
 publisher.h | 171 +
 2 files changed, 477 insertions(+)
 create mode 100644 publisher.c
 create mode 100644 publisher.h

diff --git a/publisher.c b/publisher.c
new file mode 100644
index 000..f637d23
--- /dev/null
+++ b/publisher.c
@@ -0,0 +1,306 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "publisher.h"
+#include "segment.h"
+#include 
+
+void client_log(struct Client *c)
+{
+av_log(NULL, AV_LOG_INFO, "State: ");
+switch(c->state) {
+case FREE:
+av_log(NULL, AV_LOG_INFO, "FREE\n");
+break;
+case RESERVED:
+av_log(NULL, AV_LOG_INFO, "RESERVED\n");
+break;
+case WAIT:
+av_log(NULL, AV_LOG_INFO, "WAIT\n");
+break;
+case WRITABLE:
+av_log(NULL, AV_LOG_INFO, "WRITABLE\n");
+break;
+case BUSY:
+av_log(NULL, AV_LOG_INFO, "BUSY\n");
+break;
+case BUFFER_FULL:
+av_log(NULL, AV_LOG_INFO, "BUFFER_FULL\n");
+break;
+default:
+av_log(NULL, AV_LOG_INFO, "UNKOWN\n");
+break;
+}
+}
+
+void client_disconnect(struct Client *c, int write_trailer)
+{
+struct Segment *seg;
+if (write_trailer)
+av_write_trailer(c->ofmt_ctx);
+c->ffinfo->httpd->close(c->ffinfo->server, c->ffinfo->client);
+av_free(c->ofmt_ctx->pb->buffer);
+avformat_free_context(c->ofmt_ctx);
+av_free(c->ffinfo);
+c->ofmt_ctx = NULL;
+c->ffinfo = NULL;
+pthread_mutex_lock(&c->buffer_lock);
+while(av_fifo_size(c->buffer)) {
+av_fifo_generic_read(c->buffer, &seg, sizeof(struct Segment*), NULL);
+segment_unref(seg);
+}
+pthread_mutex_unlock(&c->buffer_lock);
+client_set_state(c, FREE);
+c->current_segment_id = -1;
+}
+
+void client_set_state(struct Client *c, enum State state)
+{
+pthread_mutex_lock(&c->state_lock);
+c->state = state;
+pthread_mutex_unlock(&c->state_lock);
+}
+
+void client_push_segment(struct Client *c, struct Segment *seg)
+{
+pthread_mutex_lock(&c->buffer_lock);
+if (av_fifo_space(c->buffer) == 0) {
+av_log(NULL, AV_LOG_WARNING, "Client buffer full, dropping 
Segment.\n");
+client_set_state(c, BUFFER_FULL);
+pthread_mutex_unlock(&c->buffer_lock);
+return;
+}
+segment_ref(seg);
+av_fifo_generic_write(c->buffer, &seg, sizeof(struct Segment*), NULL);
+pthread_mutex_unlock(&c->buffer_lock);
+client_set_state(c, WRITABLE);
+}
+
+void publisher_init(struct PublisherContext **pub)
+{
+int i;
+struct PublisherContext *pc = (struct PublisherContext*) 
av_malloc(sizeof(struct PublisherContext));
+pc->nb_threads = 4;
+pc->current_segment_id = -1;
+pc->shutdown = 0;
+pc->buffer = av_fifo_alloc_array(sizeof(struct Segment), MAX_SEGMENTS);
+pc->fs_buffer = av_fifo_alloc_array(sizeof(struct Segment), MAX_SEGMENTS);
+pthread_mutex_init(&pc->buffer_lock, NULL);
+pthread_mutex_init(&pc->fs_buffer_lock, NULL);
+for (i = 0; i < MAX_CLIENTS; i++) {
+struct Client *c = &pc->clients[i];
+c->buffer = av_fifo_alloc_array(sizeof(struct Segment), MAX_SEGMENTS);
+c->ofmt_ctx = NULL;
+c->ffinfo = NULL;
+c->id = i;
+c->current_segment_id = -1;
+pthread_mutex_init(&c->state_lock, NULL);
+pthread_mutex_init(&c->buffer_lock, NULL);
+client_set_state(c, FREE);
+}
+*pub = pc;
+}
+
+void publisher_push_segment(struct PublisherContext *pub, struct Segment *seg)
+{
+struct Segment *drop;
+pthread_mutex_lock(&pub->buffer_lock);
+pthread_mutex_lock(&pub->fs_buffer_lock);
+av_fifo_generic_write(pub->buffer, &seg, sizeof(struct Segment*), NULL);
+segment_ref(seg);
+if (av_fifo_size(pub->fs_buffer) >= BUFFER_SEGMENTS * sizeof(struct 
Segment*)) {
+av_fifo_generic_read(pub->fs_buffer, &drop, sizeof(struct Segment*), 
NULL);
+segment_unref(drop);
+}
+av_fifo_generic_write(pub->fs_buffer, &seg, sizeof(struct Segment*), NULL);
+pthread_mutex_unlock(&pub->buffer_lock);
+pthread_mutex_unlock(&pub->fs_buffer_lock);
+segment_ref(seg);

[FFmpeg-devel] [PATCH 4/5] ffserver: Implement ffserver and add Makefile

[Stephan Holljes]

Signed-off-by: Stephan Holljes 
---
 Makefile   |  18 +++
 ffserver.c | 514 +
 2 files changed, 532 insertions(+)
 create mode 100644 Makefile
 create mode 100644 ffserver.c

diff --git a/Makefile b/Makefile
new file mode 100644
index 000..118ba27
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,18 @@
+all: ffserver
+LAV_FLAGS = $(shell pkg-config --libs --cflags libavformat libavcodec 
libavutil)
+# LAV_FLAGS = -L/usr/local/lib -lavcodec -lavformat -lavutil
+
+ffserver: segment.o publisher.o lavfhttpd.o ffserver.c
+   cc -g -Wall $(LAV_FLAGS) -lpthread -o ffserver segment.o publisher.o 
lavfhttpd.o ffserver.c
+
+segment.o: segment.c segment.h
+   cc -g -Wall $(LAV_FLAGS) -lpthread -c segment.c
+
+publisher.o: publisher.c publisher.h
+   cc -g -Wall $(LAV_FLAGS) -lpthread -c publisher.c
+
+lavfhttpd.o: lavfhttpd.c httpd.h
+   cc -g -Wall $(LAV_FLAGS) -lpthread -c lavfhttpd.c
+
+clean:
+   rm *.o ffserver
diff --git a/ffserver.c b/ffserver.c
new file mode 100644
index 000..9cbf469
--- /dev/null
+++ b/ffserver.c
@@ -0,0 +1,514 @@
+/*
+ * Copyright (c) 2018 Stephan Holljes
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * multimedia server based on the FFmpeg libraries
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "segment.h"
+#include "publisher.h"
+#include "httpd.h"
+
+#define BUFFER_SECS 30
+#define LISTEN_TIMEOUT_MSEC 1000
+
+struct ReadInfo {
+struct PublisherContext *pub;
+AVFormatContext *ifmt_ctx;
+char *in_filename;
+};
+
+struct WriteInfo {
+struct PublisherContext *pub;
+int thread_id;
+};
+
+struct AcceptInfo {
+struct PublisherContext *pub;
+struct HTTPDInterface *httpd;
+AVFormatContext *ifmt_ctx;
+};
+
+
+int ffserver_write(void *opaque, unsigned char *buf, int buf_size)
+{
+struct FFServerInfo *info = (struct FFServerInfo*) opaque;
+return info->httpd->write(info->server, info->client, buf, buf_size);
+}
+
+
+void *read_thread(void *arg)
+{
+struct ReadInfo *info = (struct ReadInfo*) arg;
+AVFormatContext *ifmt_ctx = info->ifmt_ctx;
+int ret, i;
+int video_idx = -1;
+int id = 0;
+int64_t pts, now, start;
+int64_t *ts;
+struct Segment *seg = NULL;
+AVPacket pkt;
+AVStream *in_stream;
+AVRational tb;
+tb.num = 1;
+tb.den = AV_TIME_BASE;
+AVStream *stream;
+AVCodecParameters *params;
+enum AVMediaType type;
+
+if ((ret = avformat_find_stream_info(ifmt_ctx, NULL)) < 0) {
+av_log(ifmt_ctx, AV_LOG_ERROR, "Could not get input stream info.\n");
+goto end;
+}
+
+av_log(ifmt_ctx, AV_LOG_INFO, "Finding video stream.\n");
+for (i = 0; i < ifmt_ctx->nb_streams; i++) {
+av_log(ifmt_ctx, AV_LOG_DEBUG, "Checking stream %d\n", i);
+stream = ifmt_ctx->streams[i];
+params = stream->codecpar;
+type = params->codec_type;
+if (type == AVMEDIA_TYPE_VIDEO) {
+video_idx = i;
+break;
+}
+}
+if (video_idx == -1) {
+av_log(ifmt_ctx, AV_LOG_ERROR, "No video stream found.\n");
+goto end;
+}
+
+
+// All information needed to start segmenting the file is gathered now.
+// start BUFFER_SECS seconds "in the past" to "catch up" to real-time. Has 
no effect on streamed sources.
+start = av_gettime_relative() - BUFFER_SECS * AV_TIME_BASE;
+
+// segmenting main-loop
+
+for (;;) {
+ret = av_read_frame(ifmt_ctx, &pkt);
+if (ret < 0)
+break;
+
+in_stream = ifmt_ctx->streams[pkt.stream_index];
+if (pkt.pts == AV_NOPTS_VALUE) {
+pkt.pts = 0;
+}
+if (pkt.dts == AV_NOPTS_VALUE) {
+pkt.dts = 0;
+}
+
+// current pts
+pts = av_rescale_q(pkt.pts, in_stream->time_base, tb);
+
+// current stream "uptime"
+now = av_gettime_relative() - start;
+
+// simulate real-time reading
+while (pts > now) {
+usleep(1000);
+now = av_gettime_relative() - start;
+}
+
+// keyfram

[FFmpeg-devel] [PATCH 3/5] ffserver: Implement http interface and implementation

[Stephan Holljes]

Signed-off-by: Stephan Holljes 
---
 httpd.h |  58 +++
 lavfhttpd.c | 150 
 2 files changed, 208 insertions(+)
 create mode 100644 httpd.h
 create mode 100644 lavfhttpd.c

diff --git a/httpd.h b/httpd.h
new file mode 100644
index 000..6fb91bd
--- /dev/null
+++ b/httpd.h
@@ -0,0 +1,58 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef HTTPDINTERFACE_H
+#define HTTPDINTERFACE_H
+
+#define HTTPD_OK 0
+#define HTTPD_LISTEN_TIMEOUT -1
+#define HTTPD_CLIENT_ERROR -2
+#define HTTPD_OTHER_ERROR -3
+
+#include "publisher.h"
+
+/* HTTPD Config struct */
+struct HTTPDConfig {
+char *bind_address;
+int port;
+int accept_timeout;
+};
+
+/* HTTPClient struct, this information is shared between ffserver and the 
httpd implementation */
+struct HTTPClient {
+/* the method requested by the client, this field has to be set and freed 
by the httpd implementation */
+char *method;
+/* the resource requested by the client, this field has to be set and 
freed by the httpd implementation */
+char *resource;
+void *httpd_data; // httpd implementation specific data
+};
+
+/* HTTPDInterface that an httpd implementation must provide */
+struct HTTPDInterface {
+int (*init)  (void **server, struct HTTPDConfig config);
+int (*free)  (void *server);
+int (*accept)(void *server, struct HTTPClient **client, int reply_code);
+int (*write) (void *server, struct HTTPClient *client, const unsigned char 
*buf, int size);
+int (*read)  (void *server, struct HTTPClient *client, unsigned char *buf, 
int size);
+void (*close)(void *server, struct HTTPClient *client);
+void (*shutdown)(void *server);
+};
+
+/* Current HTTPDInterface implementation using lavformat */
+extern struct HTTPDInterface lavfhttpd;
+#endif
diff --git a/lavfhttpd.c b/lavfhttpd.c
new file mode 100644
index 000..41f6d3e
--- /dev/null
+++ b/lavfhttpd.c
@@ -0,0 +1,150 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+ 
+#ifndef LAVFHTTPD_H
+#define LAVFHTTPD_H
+
+#include "httpd.h"
+#include 
+
+
+int lavfhttpd_init(void **server, struct HTTPDConfig config)
+{
+char out_uri[1024];
+int ret;
+AVDictionary *opts = NULL;
+AVIOContext *server_ctx = NULL;
+
+snprintf(out_uri, 1024, "http://%s:%d";, config.bind_address, config.port);
+
+avformat_network_init();
+
+if ((ret = av_dict_set(&opts, "listen", "2", 0)) < 0) {
+av_log(opts, AV_LOG_ERROR, "Failed to set listen mode for server: 
%s\n", av_err2str(ret));
+av_free(opts);
+return -1;
+}
+
+if ((ret = av_dict_set_int(&opts, "listen_timeout", config.accept_timeout, 
0)) < 0) {
+av_log(opts, AV_LOG_ERROR, "Failed to set listen_timeout for server: 
%s\n", av_err2str(ret));
+av_free(opts);
+return -1;
+}
+
+if ((ret = avio_open2(&server_ctx, out_uri, AVIO_FLAG_WRITE, NULL, &opts)) 
< 0) {
+av_log(server, AV_LOG_ERROR, "Failed to open server: %s\n", 
av_err2str(ret));
+av_free(opts);
+return -1;
+}
+av_free(opts);
+
+*server = server_ctx;
+return 0;
+}
+
+int lavfhttpd_accept(void *server, struct HTTPClient **client, int reply_code)
+{
+AVIOContext *server_ctx = (AVIOContext*) server;
+AVIOContext *client_ctx = NULL;
+struct HTTPClient *client_http = NULL;
+int ret, ret2, handshake;
+int reply_code2 = reply_code;
+char *method, *resource;
+if ((ret = avio_ac

[FFmpeg-devel] [PATCH 5/5] ffserver: Add basic documentation of the architecture

[Stephan Holljes]

Signed-off-by: Stephan Holljes 
---
 Documentation.txt | 84 +++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation.txt

diff --git a/Documentation.txt b/Documentation.txt
new file mode 100644
index 000..5b52c89
--- /dev/null
+++ b/Documentation.txt
@@ -0,0 +1,84 @@
+About
+-
+
+In its current for this is a HTTP live-streaming server. A media file can be
+streamed to a number of clients with the correct media file being muxed on-
+the-fly for each client starting at the current position in the stream.
+The stream received by the clients is simply a HTTP response to an HTTP
+request.
+
+
+Documentation
+-
+
+The current implementation has three different types of work that is done in
+different threads. These types are: reading a stream, accepting HTTP
+connections and writing media data to clients.
+
+The design tries to follow a Publisher-Subscriber-Pattern. The PublisherContext
+struct contains buffers of read media data and the list of clients. Clients
+themselves contain a buffer of media data that still has to be sent to them.
+
+The reading thread takes care of segmenting the stream into independent chunks
+of data and pushing it to the PublisherContext, which publishes the new Segment
+to connected clients. This publishing only adds this Segment to the client's
+buffer.
+
+The writing thread does the actual writing of data over the network. It checks
+each client's state and if there is data available that can be written to that
+client it is sent.
+
+The accept thread accepts new clients over HTTP and if not all client slots are
+in use, writes the stream-header and adds the client to the PublisherContext.
+
+A Segment is only stored in memory once and is refcounted. Buffers in the
+PublisherContext and clients contain pointers to Segments.
+
+Buffers are implemented using AVFifoBuffer.
+
+Client states are protected by pthread-mutex-locks, making it possible to run
+multiple write threads.
+
+HTTPD-API
+-
+
+To be independent of a specific http server implementation, an interface is
+provided that an http server implementation has to provide. At the time of
+writing an implementation using the libavformat http server is provided.
+
+The HTTPDInterface struct takes the following function pointers:
+
+struct HTTPDInterface {
+int (*init)  (void **server, struct HTTPDConfig config);
+int (*free)  (void *server);
+int (*accept)(void *server, struct HTTPClient **client, int reply_code);
+int (*write) (void *server, struct HTTPClient *client, const unsigned char 
*buf, int size);
+int (*read)  (void *server, struct HTTPClient *client, unsigned char *buf, 
int size);
+void (*close)(void *server, struct HTTPClient *client);
+void (*shutdown)(void *server);
+};
+
+
+Usage
+-
+
+Currently streams can be supplied as a stream through stdin or any ffmpeg-
+compatible URI, e.g. files or network locations. Examples:
+
+cat somefile.mkv | ./ffserver
+
+./ffserver somefile.mkv
+
+./ffserver http://somehost/somefile.mkv
+
+This will start reading the file and open port 8080 for HTTP client 
connections.
+The stream is read in real time from whatever resource it is retrieved.
+Currently a maximum of 16 clients is implemented.
+
+The server responds to any GET request with the mediastream. Any other request
+is answered with a HTTP 400 error.
+If the maximum number of clients is reached the server responds with a 503 HTTP
+error if a new client wants to connect.
+
+Once the stream ends the server will write all the remaining data to all
+connected clients before closing the connections and exiting.
-- 
2.16.2

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

[FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

These demuxers have probes that mainly probe based on file extension,
and map to codec IDs that render text as video. The result is that
ffmpeg will, by default, happily render, for example, .txt files
as images. This is not exactly a good security practice, an only
makes it easier for potential attackers to gain the contents of
system files.

Disable building these by default.

Signed-off-by: Derek Buitenhuis 
---
I've been hard disabling these at $dayjob for a long time, after some
"interesting" upload attempts, but it should probably be done for
everyone.

I'm not overly attached implementaion details like the option name
or whether it's done at build time ot runtime, but I think the concept
of "don't render arbitrary system text files" is an important one.
---
 Changelog | 1 +
 configure | 7 +++
 tests/fate.sh | 1 +
 3 files changed, 9 insertions(+)

diff --git a/Changelog b/Changelog
index d442ced..e3f8e83 100644
--- a/Changelog
+++ b/Changelog
@@ -6,6 +6,7 @@ version :
 - tmix filter
 - amplify filter
 - fftdnoiz filter
+- unsafe demuxers that render text files now disabled by default
 
 
 version 4.0:
diff --git a/configure b/configure
index a1f13a7..2f2805e 100755
--- a/configure
+++ b/configure
@@ -107,6 +107,7 @@ Configuration options:
   --enable-small   optimize for size instead of speed
   --disable-runtime-cpudetect disable detecting CPU capabilities at runtime 
(smaller binary)
   --enable-grayenable full grayscale support (slower color)
+  --enable-unsafe-demuxers enable unsafe-by-default demuxers
   --disable-swscale-alpha  disable alpha channel support in swscale
   --disable-alldisable building components, libraries and programs
   --disable-autodetect disable automatically detected external libraries 
[no]
@@ -1784,6 +1785,7 @@ FEATURE_LIST="
 small
 static
 swscale_alpha
+unsafe_demuxers
 "
 
 LIBRARY_LIST="
@@ -3100,6 +3102,7 @@ videotoolbox_encoder_deps="videotoolbox 
VTCompressionSessionPrepareToEncodeFrame
 
 # demuxers / muxers
 ac3_demuxer_select="ac3_parser"
+adf_demuxer_deps="unsafe_demuxers"
 aiff_muxer_select="iso_media"
 asf_demuxer_select="riffdec"
 asf_o_demuxer_select="riffdec"
@@ -3107,6 +3110,7 @@ asf_muxer_select="riffenc"
 asf_stream_muxer_select="asf_muxer"
 avi_demuxer_select="iso_media riffdec exif"
 avi_muxer_select="riffenc"
+bintext_demuxer_deps="unsafe_demuxers"
 caf_demuxer_select="iso_media riffdec"
 caf_muxer_select="iso_media"
 dash_muxer_select="mp4_muxer"
@@ -3124,6 +3128,7 @@ flac_demuxer_select="flac_parser"
 hds_muxer_select="flv_muxer"
 hls_muxer_select="mpegts_muxer"
 hls_muxer_suggest="gcrypt openssl"
+idf_demuxer_deps="unsafe_demuxers"
 image2_alias_pix_demuxer_select="image2_demuxer"
 image2_brender_pix_demuxer_select="image2_demuxer"
 ipod_muxer_select="mov_muxer"
@@ -3167,6 +3172,7 @@ swf_demuxer_suggest="zlib"
 tak_demuxer_select="tak_parser"
 tg2_muxer_select="mov_muxer"
 tgp_muxer_select="mov_muxer"
+tty_demuxer_deps="unsafe_demuxers"
 vobsub_demuxer_select="mpegps_demuxer"
 w64_demuxer_select="wav_demuxer"
 w64_muxer_select="wav_muxer"
@@ -3176,6 +3182,7 @@ webm_muxer_select="iso_media riffenc"
 webm_dash_manifest_demuxer_select="matroska_demuxer"
 wtv_demuxer_select="mpegts_demuxer riffdec"
 wtv_muxer_select="mpegts_muxer riffenc"
+xbin_demuxer_deps="unsafe_demuxers"
 xmv_demuxer_select="riffdec"
 xwma_demuxer_select="riffdec"
 
diff --git a/tests/fate.sh b/tests/fate.sh
index 0edee7f..6a99d66 100755
--- a/tests/fate.sh
+++ b/tests/fate.sh
@@ -49,6 +49,7 @@ configure()(
 --enable-gpl\
 --enable-memory-poisoning   \
 --enable-avresample \
+--enable-unsafe-demuxers\
 ${ignore_tests:+--ignore-tests="$ignore_tests"} \
 ${arch:+--arch=$arch}   \
 ${cpu:+--cpu="$cpu"}\
-- 
1.8.3.1

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Paul B Mahol]

On 5/10/18, Derek Buitenhuis  wrote:
> These demuxers have probes that mainly probe based on file extension,
> and map to codec IDs that render text as video. The result is that
> ffmpeg will, by default, happily render, for example, .txt files
> as images. This is not exactly a good security practice, an only
> makes it easier for potential attackers to gain the contents of
> system files.
>
> Disable building these by default.
>
> Signed-off-by: Derek Buitenhuis 
> ---
> I've been hard disabling these at $dayjob for a long time, after some
> "interesting" upload attempts, but it should probably be done for
> everyone.
>
> I'm not overly attached implementaion details like the option name
> or whether it's done at build time ot runtime, but I think the concept
> of "don't render arbitrary system text files" is an important one.
> ---
>  Changelog | 1 +
>  configure | 7 +++
>  tests/fate.sh | 1 +
>  3 files changed, 9 insertions(+)
>
> diff --git a/Changelog b/Changelog
> index d442ced..e3f8e83 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,7 @@ version :
>  - tmix filter
>  - amplify filter
>  - fftdnoiz filter
> +- unsafe demuxers that render text files now disabled by default
>

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Rostislav Pehlivanov]

On 10 May 2018 at 16:44, Derek Buitenhuis 
wrote:

> These demuxers have probes that mainly probe based on file extension,
> and map to codec IDs that render text as video. The result is that
> ffmpeg will, by default, happily render, for example, .txt files
> as images. This is not exactly a good security practice, an only
> makes it easier for potential attackers to gain the contents of
> system files.
>
> Disable building these by default.
>
> Signed-off-by: Derek Buitenhuis 
> ---
> I've been hard disabling these at $dayjob for a long time, after some
> "interesting" upload attempts, but it should probably be done for
> everyone.
>
> I'm not overly attached implementaion details like the option name
> or whether it's done at build time ot runtime, but I think the concept
> of "don't render arbitrary system text files" is an important one.
> ---
>  Changelog | 1 +
>  configure | 7 +++
>  tests/fate.sh | 1 +
>  3 files changed, 9 insertions(+)
>
> diff --git a/Changelog b/Changelog
> index d442ced..e3f8e83 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,7 @@ version :
>  - tmix filter
>  - amplify filter
>  - fftdnoiz filter
> +- unsafe demuxers that render text files now disabled by default
>
>
>  version 4.0:
> diff --git a/configure b/configure
> index a1f13a7..2f2805e 100755
> --- a/configure
> +++ b/configure
> @@ -107,6 +107,7 @@ Configuration options:
>--enable-small   optimize for size instead of speed
>--disable-runtime-cpudetect disable detecting CPU capabilities at
> runtime (smaller binary)
>--enable-grayenable full grayscale support (slower color)
> +  --enable-unsafe-demuxers enable unsafe-by-default demuxers
>--disable-swscale-alpha  disable alpha channel support in swscale
>--disable-alldisable building components, libraries and
> programs
>--disable-autodetect disable automatically detected external
> libraries [no]
> @@ -1784,6 +1785,7 @@ FEATURE_LIST="
>  small
>  static
>  swscale_alpha
> +unsafe_demuxers
>  "
>
>  LIBRARY_LIST="
> @@ -3100,6 +3102,7 @@ videotoolbox_encoder_deps="videotoolbox
> VTCompressionSessionPrepareToEncodeFrame
>
>  # demuxers / muxers
>  ac3_demuxer_select="ac3_parser"
> +adf_demuxer_deps="unsafe_demuxers"
>  aiff_muxer_select="iso_media"
>  asf_demuxer_select="riffdec"
>  asf_o_demuxer_select="riffdec"
> @@ -3107,6 +3110,7 @@ asf_muxer_select="riffenc"
>  asf_stream_muxer_select="asf_muxer"
>  avi_demuxer_select="iso_media riffdec exif"
>  avi_muxer_select="riffenc"
> +bintext_demuxer_deps="unsafe_demuxers"
>  caf_demuxer_select="iso_media riffdec"
>  caf_muxer_select="iso_media"
>  dash_muxer_select="mp4_muxer"
> @@ -3124,6 +3128,7 @@ flac_demuxer_select="flac_parser"
>  hds_muxer_select="flv_muxer"
>  hls_muxer_select="mpegts_muxer"
>  hls_muxer_suggest="gcrypt openssl"
> +idf_demuxer_deps="unsafe_demuxers"
>  image2_alias_pix_demuxer_select="image2_demuxer"
>  image2_brender_pix_demuxer_select="image2_demuxer"
>  ipod_muxer_select="mov_muxer"
> @@ -3167,6 +3172,7 @@ swf_demuxer_suggest="zlib"
>  tak_demuxer_select="tak_parser"
>  tg2_muxer_select="mov_muxer"
>  tgp_muxer_select="mov_muxer"
> +tty_demuxer_deps="unsafe_demuxers"
>  vobsub_demuxer_select="mpegps_demuxer"
>  w64_demuxer_select="wav_demuxer"
>  w64_muxer_select="wav_muxer"
> @@ -3176,6 +3182,7 @@ webm_muxer_select="iso_media riffenc"
>  webm_dash_manifest_demuxer_select="matroska_demuxer"
>  wtv_demuxer_select="mpegts_demuxer riffdec"
>  wtv_muxer_select="mpegts_muxer riffenc"
> +xbin_demuxer_deps="unsafe_demuxers"
>  xmv_demuxer_select="riffdec"
>  xwma_demuxer_select="riffdec"
>
> diff --git a/tests/fate.sh b/tests/fate.sh
> index 0edee7f..6a99d66 100755
> --- a/tests/fate.sh
> +++ b/tests/fate.sh
> @@ -49,6 +49,7 @@ configure()(
>  --enable-gpl\
>  --enable-memory-poisoning   \
>  --enable-avresample \
> +--enable-unsafe-demuxers\
>  ${ignore_tests:+--ignore-tests="$ignore_tests"} \
>  ${arch:+--arch=$arch}   \
>  ${cpu:+--cpu="$cpu"}\
> --
> 1.8.3.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Could you send a patch to disable the decoders as well?
Looks good otherwise.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Jan Ekström]

On Thu, May 10, 2018 at 6:53 PM, Paul B Mahol  wrote:
>
> Against.

Hi,

Thank you for your review.

I would recommend you put a bit more effort into explaining which
parts you are opposed to. Think of yourself being on the receiving end
of such comments, how you would like to just get a flat out refusal
without any further details. You do not give the other side any chance
of doing anything, because you have not noted what is wrong with it.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Paul B Mahol]

On 5/10/18, Jan Ekstroem  wrote:
> On Thu, May 10, 2018 at 6:53 PM, Paul B Mahol  wrote:
>>
>> Against.
>
> Hi,
>
> Thank you for your review.
>
> I would recommend you put a bit more effort into explaining which
> parts you are opposed to. Think of yourself being on the receiving end
> of such comments, how you would like to just get a flat out refusal
> without any further details. You do not give the other side any chance
> of doing anything, because you have not noted what is wrong with it.

I do not like it being disabled by default.

There are options to disable compilation of such modules already.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Thu, May 10, 2018 at 4:55 PM, Rostislav Pehlivanov
 wrote:
> Could you send a patch to disable the decoders as well?
> Looks good otherwise.

Yeah, I thought about doing that too. I can add that, though the option
will have to be renamed to something else.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Thu, May 10, 2018 at 5:11 PM, Paul B Mahol  wrote:
> I do not like it being disabled by default.

With all due respect, this is not a valid technical argument, in any respect.

> There are options to disable compilation of such modules already.

We should be defaulting to 'safe/sane by default'. Insecure and insane
defaults are
the cause of a huge number of issues across the web. The onus should not be on
the user to disable/fix all the bad defaults. That's how you become PHP.

- Derek

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

Re: [FFmpeg-devel] [PATCH] avcodec/hevcdec: make ff_hevc_frame_nb_refs take a const pointer

[Timo Rothenpieler]

applied and backported to 4.0



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/nvdec_hevc: Fix scaling lists

[Timo Rothenpieler]

applied and backported to 4.0



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/nvdec: avoid needless copy of output frame

[Oscar Amoros Huguet]

Just want to update,

We compiled master branch with this patches, tested and looked at NSIGHT.

Everything correct: 
- The internal NVDEC kernel is using the stream we have set in 
AVCUDADeviceContext struct.
- It slightly overlaps with other kernels, removing wait times between kernel 
executions.
- Nice that the Device to Device copies disappeared.

We are updating our ffmpeg compilation to this master branch commit.

It was a pleasure to collaborate.

Oscar

-Original Message-
From: ffmpeg-devel  On Behalf Of Timo 
Rothenpieler
Sent: Thursday, May 10, 2018 12:52 AM
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 2/6] avcodec/nvdec: avoid needless copy of 
output frame

applied
follow-up CUstream patches also applied

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Marton Balint]

On Thu, 10 May 2018, Derek Buitenhuis wrote:


These demuxers have probes that mainly probe based on file extension,
and map to codec IDs that render text as video. The result is that
ffmpeg will, by default, happily render, for example, .txt files
as images. This is not exactly a good security practice, an only
makes it easier for potential attackers to gain the contents of
system files.


Maybe it is better if we simply get rid of the "probing" part, so the user 
would have to explicitly specify the demuxer to use them.


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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Gyan Doshi]

On 5/10/2018 11:17 PM, Marton Balint wrote:


Maybe it is better if we simply get rid of the "probing" part, so the 
user would have to explicitly specify the demuxer to use them.


+1

Maybe shift such probes to inside a block and add a new generic lavf 
option to set whether such probes are enabled. Default would to require 
user-set demuxer.


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

Re: [FFmpeg-devel] avformat/mpegts: set AV_DISPOSITION_DESCRIPTIONS for OIPF cases

[Aman Gupta]

On Thu, May 10, 2018 at 1:46 AM Łukasz Krzciuk  wrote:

> Lets skip a testcase. It will be not easy to me to prepare it.


It should be simple to extract a small sample containing the PMT. If you
link some larger samples I can help extract the relevant bytes for test
case.

Aman


>
> Regards,
>
> *Łukasz Krzciuk*
> Developer
>
> Vewd
> ul. Grabarska 2
> , Pegaz 2A,
> 50-079 Wrocław, Polska
>
>
> On Thu, May 10, 2018 at 3:09 AM, Michael Niedermayer
>  > wrote:
>
> > On Wed, May 09, 2018 at 10:19:26AM +0200, Łukasz Krzciuk wrote:
> > > Any updates on this issue?
> >
> > about adding a testcase ?
> > adding a huge sample file is not possible.
> > It either has to be made smaller or generated by a muxer
> > or last resort, no test
> >
> > [...]
> > --
> > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > The worst form of inequality is to try to make unequal things equal.
> > -- Aristotle
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] hwcontext_vaapi: Add an assert in vaapi_map_from_drm()

[Mark Thompson]

On 10/05/18 04:08, Haihao Xiang wrote:
> The flag of input_available must be set when pic_start is not NULL, so
> add an assert to ensure it is true. In addition, the assert on last_pic
> is unnecessary now, so remove this assert.
> 
> Signed-off-by: Haihao Xiang 
> ---
>  libavcodec/vaapi_encode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

On 10/05/18 04:08, Haihao Xiang wrote:
> Every fourcc in vaapi_drm_format_map should be in
> vaapi_format_map, so add an assert to ensure we have the right maps.
> 
> Signed-off-by: Haihao Xiang 
> ---
>  libavutil/hwcontext_vaapi.c | 2 ++
>  1 file changed, 2 insertions(+)

Both applied.

Thanks,

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Thu, May 10, 2018 at 6:52 PM, Gyan Doshi  wrote:
> On 5/10/2018 11:17 PM, Marton Balint wrote:
>> Maybe it is better if we simply get rid of the "probing" part, so the user
>> would have to explicitly specify the demuxer to use them.
>
> +1
>
> Maybe shift such probes to inside a block and add a new generic lavf option
> to set whether such probes are enabled. Default would to require user-set
> demuxer.

I don't object to either approach.

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mpegts: parse sections with multiple tables

[Michael Niedermayer]

On Wed, May 09, 2018 at 02:35:00PM -0700, Aman Gupta wrote:
> From: Aman Gupta 
> 
> Fixes PMT parsing in some mpegts streams which contain
> multiple tables within the PMT pid. Previously, the parser
> assumed only one table was present in each packet, and discarded
> the rest of the section data after attempting to parse the first
> table.
> 
> A similar issue was documented in the BeyondTV software[1], which
> helped me diagnose the same bug in the ffmpeg mpegts demuxer. I also
> tried DVBInspector, libdvbpsi's dvbinfo, and tstools' tsinfo to
> help debug. The former two properly read PMTs with multiple tables,
> whereas the last has the same bug as ffmpeg.
> 
> I've created a minimal sample[2] which contains the combined PMT.
> Here's what ffmpeg probe shows before and after this patch:
> 
> Before:
> 
> Input #0, mpegts, from 'combined-pmt-tids.ts':
>   Duration: 00:00:01.08, start: 4932.966167, bitrate: 741 kb/s
>   Program 1
>   No Program
> Stream #0:0[0xf9d]: Audio: ac3, 48000 Hz, mono, fltp, 96 kb/s
> Stream #0:1[0xf9b]: Audio: mp3, 0 channels, fltp
> Stream #0:2[0xf9c]: Unknown: none
> 
> After:
> 
> Input #0, mpegts, from 'combined-pmt-tids.ts':
>   Duration: 00:00:01.11, start: 4932.966167, bitrate: 718 kb/s
>   Program 1
> Stream #0:0[0xf9b]: Video: mpeg2video ([2][0][0][0] / 0x0002), 
> none(tv, top first), 29.97 fps, 29.97 tbr, 90k tbn, 90k tbc
> Stream #0:1[0xf9c](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, 
> 5.1(side), fltp, 384 kb/s
> Stream #0:2[0xf9d](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, 
> mono, fltp, 96 kb/s
> 
> With the patch, the PMT is parsed correctly so the streams are
> created in the correct order, are associated with "Program 1",
> and their codecs are set correctly.
> 
> [1] http://forums.snapstream.com/vb/showpost.php?p=343816&postcount=201
> [2] https://s3.amazonaws.com/tmm1/combined-pmt-tids.ts
> 
> Signed-off-by: Aman Gupta 
> ---
>  libavformat/mpegts.c | 26 +++---
>  1 file changed, 19 insertions(+), 7 deletions(-)

probably ok.

The code looks a bit convoluted, so testing with a fuzzer probably wont hurt

thx

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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 1/4] cbs_h265: read/write HEVC PREFIX SEI

[Mark Thompson]

On 10/05/18 06:47, Haihao Xiang wrote:
> Similar to H264, cbs_h265_{read, write}_nal_unit() can handle HEVC
> prefix SEI NAL units. Currently mastering display colour volume SEI
> message is added only, we may add more SEI message if needed later
> 
> v2: Fix coding style and rebase the code
> 
> v3: Fix assertion failure when running the command below:
> ffmpeg -i input-with-sei.mkv -c:v copy -bsf:v trace_headers -f null -
> Fix the ranges for max_display_mastering_luminance and 
> min_display_mastering_luminance
> 
> Signed-off-by: Haihao Xiang 
> ---
>  libavcodec/cbs_h2645.c|  45 ++
>  libavcodec/cbs_h265.h |  36 
>  libavcodec/cbs_h265_syntax_template.c | 158 
> ++
>  3 files changed, 239 insertions(+)
> 
> ...
>  
> +typedef struct H265RawSEIMasteringDiplayColourVolume {

I fixed this typo ^^ which appears in several places (a bad 
feature of code completion, I guess :P ).

Then tested, all looks good, applied.

Thanks!

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

Re: [FFmpeg-devel] [PATCH 2/3] libavcodec: v4l2m2m: output AVDRMFrameDescriptor

[Jorge Ramirez-Ortiz]

On 05/10/2018 04:47 PM, wm4 wrote:

so what would be the sequence of calls for an libavcodec client to
request the DRM format?
https://github.com/BayLibre/ffmpeg-drm/blob/master/main.c#L598

Like in 100% of all existing cases in ffmpeg: set a get_format
callback, and return the format from it.


Sure, but it is seems like a bit of a unortodox solution to what in 
principle seems like a simple problem of passing a list of valid 
formats; a solution that allows clients to sneak in all sort of 
additional actions and just confuse the code.


Just looking at doc/examples/qsvdec.c, get_format not only returns the 
format but also allocates hw frames; so it seems like single 
responsibilities are not necessarily associated to this callback then 
(ie, get_format is not only about getting formats is it?).





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

Re: [FFmpeg-devel] [PATCH v3] avformat/mxfenc: add h264 profiles

[Tomas Härdin]

tis 2018-05-08 klockan 18:32 +0200 skrev Thomas Mundt:
> 2018-05-07 10:40 GMT+02:00 Tomas Härdin :
> 
> > sön 2018-05-06 klockan 21:31 +0200 skrev Thomas Mundt:
> > > 2018-05-06 13:32 GMT+02:00 Tomas Härdin :
> > > 
> > > > fre 2018-05-04 klockan 01:52 +0200 skrev Thomas Mundt:
> > > > > Hi,
> > > > > 
> > > > > this is a better version of the patch.
> > > > > 10 bit and TFF are mandatory for AVC Intra only. Other
> > > > > profiles
> > > > > differ.
> > > > > 
> > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > > > index 3bb7032..81513dc 100644
> > > > > --- a/libavformat/mxfenc.c
> > > > > +++ b/libavformat/mxfenc.c
> > > > > @@ -1947,22 +1947,31 @@ static const struct {
> > > > >  int frame_size;
> > > > >  int profile;
> > > > >  uint8_t interlaced;
> > > > > +int long_gop;
> > > > 
> > > > A comment here explaining the difference between -1, 0 and 1
> > > > would
> > > > be
> > > > nice. The rest looks OK, but I didn't read the relevant specs
> > > > to be
> > > > 100% sure
> > > > 
> > > > 
> > > 
> > > New patch attached.
> > 
> > Looks OK
> 
> 
> Thanks,
> 
> can you or someone else push it please.

Could you attach a patch that applies cleanly? I tried to git am it on
commit 0612e29 but not luck

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Michael Niedermayer]

On Thu, May 10, 2018 at 04:44:59PM +0100, Derek Buitenhuis wrote:
> These demuxers have probes that mainly probe based on file extension,
> and map to codec IDs that render text as video. The result is that
> ffmpeg will, by default, happily render, for example, .txt files
> as images. This is not exactly a good security practice, an only
> makes it easier for potential attackers to gain the contents of
> system files.
> 
> Disable building these by default.
> 
> Signed-off-by: Derek Buitenhuis 
> ---
> I've been hard disabling these at $dayjob for a long time, after some
> "interesting" upload attempts, but it should probably be done for
> everyone.
> 
> I'm not overly attached implementaion details like the option name
> or whether it's done at build time ot runtime, but I think the concept
> of "don't render arbitrary system text files" is an important one.
> ---
>  Changelog | 1 +
>  configure | 7 +++
>  tests/fate.sh | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/Changelog b/Changelog
> index d442ced..e3f8e83 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,7 @@ version :
>  - tmix filter
>  - amplify filter
>  - fftdnoiz filter
> +- unsafe demuxers that render text files now disabled by default

The problem and the suggested solution seem only somewhat overlapping

please correct me if iam wrong, theres quite a bit iam guessing here
IIUC the problem is that in your usecase 
1. ffmpeg has access to sensitive files
2. one of these files can be opened by an attacker with ffmpeg
2b. This involves the file being probed as a supported format
3. The file is then demuxed, decoded, encoded, muxed
4. the result is given back to the attacker

This patchset removes one of the demuxers involved in the attack

The first problem of this patchset is that it does not provide any
evidence of how the other demuxers probe functions can trigger on
random text files.
for example idf_probe() requires, the first 12 bytes of the file  to
match exactly and some of these are not text. So a attack which depends
on the probing of sensitive text data to succeed should not work with it

The second problem i see is that the patch disables the demuxers, while
disabling the probe functions affected should have the same effect
security wise but is a smaller step in respect to disabling.

The third problem i see is that really once you read sensitive data
and pass something back to the user you already lost.
A text demuxer and decoder makes it easier and it makes it much easier
to demonstrate the attack in a flashy way. But having the probe code
access the sensitive data even if none succeeds already makes quite
a bit of internal state (caches, branch prediction, deallocated memory, ...)
be contaminated with sensitive information. Its hard to ensure that
none of this can be recovered by the attacker.

I think first ffmpeg should not be able to access sensitive data.
Then none of our probe functions should succeed on the average
sensitive text file. If one does, we should look into making it not
detect that.

Also it may seem counter-intuitive but adding a probe function which
is designed to succeed on sensitive text files may be more reliable
to stop their detection than to disable probe functions.
The probe system is basically doing differential probing. That is
the one of 2 that has the higher score wins.
So it may be more effective to add a function that that returns a
high score intentionally for a null demuxer than to try to stop
every function that returns more than 0
if such probe code is added, it also could maybe warn the admin about a
potential misconfiguration that allows probing to reach to sensitive
data.


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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


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

Re: [FFmpeg-devel] [PATCH 06/13] avformat/mxfenc: Add Sample width/height/x offset/y offset, Display x offset and F2 offset

[Tomas Härdin]

tis 2018-05-08 klockan 18:26 +0200 skrev Michael Niedermayer:
> On Tue, May 08, 2018 at 12:40:49PM +0200, Tomas Härdin wrote:
> > mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
> > > > Signed-off-by: Michael Niedermayer 
> > > 
> > > +if (sc->interlaced) {
> > > +//Display F2 Offset
> > > +mxf_write_local_tag(pb, 4, 0x3217);
> > > +avio_wb32(pb, -((st->codecpar->height -
> > > display_height)&1));
> > 
> > Negative values for DisplayF2Offset are not valid
> 
> The specification (SMPTE 377-1-2009) says:
> 
> The DisplayF2Offset Property adjusts the DisplayYOffset for the
> second field relative to that for the first field. Its
> value shall be zero (0) or minus 1. A value of minus 1 shall invert
> the Displayed Topness relative to the Sampled
> Topness.

Huh, the document I have is from 2004, which says "If the property is
not present, its value shall be assumed to be 0. Valid values are zero
or 1.". So I guess they changed that. I guess it's fine then.

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

Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components

[Mark Thompson]

On 08/05/18 12:00, Alexander Kravchenko wrote:
> This patch moves AMF common parts from amfenc to hwcontext_amf.
> Now av_hwdevice_ctx API is used for AMF context creation/destroying.
> This patch does not change component behaviour.
> it contains only restructurization for further patches with new amf components
> 
> ---
>  libavcodec/amfenc.c| 250 ---
>  libavcodec/amfenc.h|  27 +---
>  libavutil/Makefile |   2 +
>  libavutil/hwcontext.c  |   4 +
>  libavutil/hwcontext.h  |   1 +
>  libavutil/hwcontext_amf.c  | 329 
> +
>  libavutil/hwcontext_amf.h  |  43 ++
>  libavutil/hwcontext_internal.h |   1 +
>  8 files changed, 418 insertions(+), 239 deletions(-)
>  create mode 100644 libavutil/hwcontext_amf.c
>  create mode 100644 libavutil/hwcontext_amf.h
> 
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 384d8efc92..a8186c7b86 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -21,13 +21,7 @@
>  #include "libavutil/avassert.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/hwcontext.h"
> -#if CONFIG_D3D11VA
> -#include "libavutil/hwcontext_d3d11va.h"
> -#endif
> -#if CONFIG_DXVA2
> -#define COBJMACROS
> -#include "libavutil/hwcontext_dxva2.h"
> -#endif
> +
>  #include "libavutil/mem.h"
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/time.h"
> @@ -35,14 +29,12 @@
>  #include "amfenc.h"
>  #include "internal.h"
>  
> -#if CONFIG_D3D11VA
> -#include 
> +#if CONFIG_DXVA2
> +#include 
>  #endif
>  
> -#ifdef _WIN32
> -#include "compat/w32dlfcn.h"
> -#else
> -#include 
> +#if CONFIG_D3D11VA
> +#include 
>  #endif
>  
>  #define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
> @@ -88,34 +80,17 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum 
> AVPixelFormat fmt)
>  return AMF_SURFACE_UNKNOWN;
>  }
>  
> -static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
> -const wchar_t *scope, const wchar_t *message)
> -{
> -AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> -av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n 
> is provided from AMF
> -}
>  
> -static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
> -{
> -}
> -
> -static AMFTraceWriterVtbl tracer_vtbl =
> -{
> -.Write = AMFTraceWriter_Write,
> -.Flush = AMFTraceWriter_Flush,
> -};
> -
> -static int amf_load_library(AVCodecContext *avctx)
> +static int amf_init_context(AVCodecContext *avctx)
>  {
> -AmfContext*ctx = avctx->priv_data;
> -AMFInit_Fn init_fun;
> -AMFQueryVersion_Fn version_fun;
> -AMF_RESULT res;
> +AmfContext *ctx = avctx->priv_data;
> +av_unused int ret;

This is now used in every configuration, so it doesn't need the annotation.

>  
>  ctx->delayed_frame = av_frame_alloc();
>  if (!ctx->delayed_frame) {
>  return AVERROR(ENOMEM);
>  }
> +

Cosmetic change, and it's also trailing whitespace.

>  // hardcoded to current HW queue size - will realloc in 
> timestamp_queue_enqueue() if too small
>  ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) * 
> sizeof(int64_t));
>  if (!ctx->timestamp_list) {
> @@ -123,119 +98,9 @@ static int amf_load_library(AVCodecContext *avctx)
>  }
>  ctx->dts_delay = 0;
>  
> -
> -ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
> -AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
> -AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
> -
> -init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
> -AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s 
> failed to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
> -
> -version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library, 
> AMF_QUERY_VERSION_FUNCTION_NAME);
> -AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s 
> failed to find function %s\n", AMF_DLL_NAMEA, 
> AMF_QUERY_VERSION_FUNCTION_NAME);
> -
> -res = version_fun(&ctx->version);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with 
> error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
> -res = init_fun(AMF_FULL_VERSION, &ctx->factory);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with 
> error %d\n", AMF_INIT_FUNCTION_NAME, res);
> -res = ctx->factory->pVtbl->GetTrace(ctx->factory, &ctx->trace);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace() 
> failed with error %d\n", res);
> -res = ctx->factory->pVtbl->GetDebug(ctx->factory, &ctx->debug);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug() 
> failed with error %d\n", res);
> -return 0;
> -}
> -
> -#if CONFIG_D3D11VA
> -static int amf_init_from_d3d11_device(AVCodecContext *avctx, 
> AVD3D11VADeviceContext *hwctx)
> -{
> -AmfContext *ctx = avctx->priv_data;
> -   

Re: [FFmpeg-devel] [PATCH v3] avformat/mxfenc: add h264 profiles

[Tomas Härdin]

tor 2018-05-10 klockan 22:44 +0200 skrev Tomas Härdin:
> tis 2018-05-08 klockan 18:32 +0200 skrev Thomas Mundt:
> > 2018-05-07 10:40 GMT+02:00 Tomas Härdin :
> > 
> > > sön 2018-05-06 klockan 21:31 +0200 skrev Thomas Mundt:
> > > > 2018-05-06 13:32 GMT+02:00 Tomas Härdin :
> > > > 
> > > > > fre 2018-05-04 klockan 01:52 +0200 skrev Thomas Mundt:
> > > > > > Hi,
> > > > > > 
> > > > > > this is a better version of the patch.
> > > > > > 10 bit and TFF are mandatory for AVC Intra only. Other
> > > > > > profiles
> > > > > > differ.
> > > > > > 
> > > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > > > > index 3bb7032..81513dc 100644
> > > > > > --- a/libavformat/mxfenc.c
> > > > > > +++ b/libavformat/mxfenc.c
> > > > > > @@ -1947,22 +1947,31 @@ static const struct {
> > > > > >  int frame_size;
> > > > > >  int profile;
> > > > > >  uint8_t interlaced;
> > > > > > +int long_gop;
> > > > > 
> > > > > A comment here explaining the difference between -1, 0 and 1
> > > > > would
> > > > > be
> > > > > nice. The rest looks OK, but I didn't read the relevant specs
> > > > > to be
> > > > > 100% sure
> > > > > 
> > > > > 
> > > > 
> > > > New patch attached.
> > > 
> > > Looks OK
> > 
> > 
> > Thanks,
> > 
> > can you or someone else push it please.
> 
> Could you attach a patch that applies cleanly? I tried to git am it
> on
> commit 0612e29 but not luck

Never mind, I just saw Michael applied this

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

Re: [FFmpeg-devel] [PATCH v3] avformat/mxfenc: add h264 profiles

[Carl Eugen Hoyos]

2018-05-10 22:44 GMT+02:00, Tomas Härdin :

> Could you attach a patch that applies cleanly? I tried to git am it on
> commit 0612e29 but not luck

Because it was applied as a3a6d4da6

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

[FFmpeg-devel] [PATCH]lavc/v210dec: Skip Canopus C210 extradata

[Carl Eugen Hoyos]

Hi!

Peter Bubestinger provided a C210 file where every frame starts with
64 bytes of extradata (24 byte "INFO", 16 byte "RDRT", rest "FIEL").
Piotr confirmed that the Canopus decoder accepts files without the
extradata but consumes it if present.
Attached patch fixes the file in question visually.

Please comment, Carl Eugen
From 42b101a00cc1d78ee6a5207ac09546b67adf7fe5 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Thu, 10 May 2018 22:45:07 +0200
Subject: [PATCH] lavc/v210dec: Skip Canopus C210 extradata.

Unbreaks files with unknown extradata.
---
 libavcodec/v210dec.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavcodec/v210dec.c b/libavcodec/v210dec.c
index 99199dd..ddc5dbe 100644
--- a/libavcodec/v210dec.c
+++ b/libavcodec/v210dec.c
@@ -27,6 +27,7 @@
 #include "libavutil/bswap.h"
 #include "libavutil/internal.h"
 #include "libavutil/mem.h"
+#include "libavutil/intreadwrite.h"
 
 #define READ_PIXELS(a, b, c) \
 do { \
@@ -92,6 +93,11 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
 return AVERROR_INVALIDDATA;
 }
 }
+if (   avctx->codec_tag == MKTAG('C', '2', '1', '0')
+&& avpkt->size > 64
+&& AV_RN32(psrc) == AV_RN32("INFO")
+&& avpkt->size - 64 >= stride * avctx->height)
+psrc += 64;
 
 aligned_input = !((uintptr_t)psrc & 0xf) && !(stride & 0xf);
 if (aligned_input != s->aligned_input) {
-- 
1.7.10.4

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

Re: [FFmpeg-devel] [PATCH] mpegvideo_enc: add option to disable intra mbs in p frames

[Michael Niedermayer]

On Wed, May 09, 2018 at 08:44:25PM +0200, Ramiro Polla wrote:
> This option prevents the mpv encoders from using intra macroblocks in
> predictive frames.
> 
> It is useful for glitch artists to generate input material. This option
> allows them to split and merge two video files while maintaining fluid
> motion from the second video without having intra macroblocks restoring
> chunks of the first video.

maybe a continuous variable like snows intra_penalty could achieve this
too but give more flexibility in doing it also just partially if wanted

either solution LGTM

thx

[...]
-- 
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

Re: [FFmpeg-devel] [PATCH] fate: add more tests for hue video filter

[Michael Niedermayer]

On Wed, May 09, 2018 at 12:05:48PM +0200, Tobias Rapp wrote:
> Adds tests for the hue angle and brightness filter parameters.
> Renames the existing saturation parameter test for consistency.
> 
> Signed-off-by: Tobias Rapp 
> ---
>  tests/fate/filter-video.mak | 10 --
>  tests/ref/fate/filter-hue   |  1 -
>  tests/ref/fate/filter-hue1  |  1 +
>  tests/ref/fate/filter-hue2  |  1 +
>  tests/ref/fate/filter-hue3  |  1 +
>  5 files changed, 11 insertions(+), 3 deletions(-)
>  delete mode 100644 tests/ref/fate/filter-hue
>  create mode 100644 tests/ref/fate/filter-hue1
>  create mode 100644 tests/ref/fate/filter-hue2
>  create mode 100644 tests/ref/fate/filter-hue3

should be ok

thx

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Carl Eugen Hoyos]

2018-05-10 17:44 GMT+02:00, Derek Buitenhuis :
> These demuxers have probes that mainly probe based on file extension,
> and map to codec IDs that render text as video. The result is that
> ffmpeg will, by default, happily render, for example, .txt files
> as images. This is not exactly a good security practice, an only
> makes it easier for potential attackers to gain the contents of
> system files.
>
> Disable building these by default.
>
> Signed-off-by: Derek Buitenhuis 
> ---
> I've been hard disabling these at $dayjob for a long time, after some
> "interesting" upload attempts, but it should probably be done for
> everyone.
>
> I'm not overly attached implementaion details like the option name
> or whether it's done at build time ot runtime, but I think the concept
> of "don't render arbitrary system text files" is an important one.

Disabling demuxers by default does not seem to be a good idea to me.

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

Re: [FFmpeg-devel] [PATCH]lavc/qdrw: Read PixMap palette

[Carl Eugen Hoyos]

2018-05-05 2:26 GMT+02:00, Carl Eugen Hoyos :

> Attached patch fixes ticket #6195 for me.

Patch applied.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[James Darnley]

On 2018-05-10 17:44, Derek Buitenhuis wrote:
> These demuxers have probes that mainly probe based on file extension,
> and map to codec IDs that render text as video. The result is that
> ffmpeg will, by default, happily render, for example, .txt files
> as images. This is not exactly a good security practice, an only
> makes it easier for potential attackers to gain the contents of
> system files.
> 
> Disable building these by default.
> 
> Signed-off-by: Derek Buitenhuis 
> ---
> I've been hard disabling these at $dayjob for a long time, after some
> "interesting" upload attempts, but it should probably be done for
> everyone.
> 
> I'm not overly attached implementaion details like the option name
> or whether it's done at build time ot runtime, but I think the concept
> of "don't render arbitrary system text files" is an important one.
> ---

You web people already have options for the various annoying whitelists.
 Is this not covered by one of them?

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

Re: [FFmpeg-devel] [PATCH 06/13] avformat/mxfenc: Add Sample width/height/x offset/y offset, Display x offset and F2 offset

[Michael Niedermayer]

On Thu, May 10, 2018 at 10:48:59PM +0200, Tomas Härdin wrote:
> tis 2018-05-08 klockan 18:26 +0200 skrev Michael Niedermayer:
> > On Tue, May 08, 2018 at 12:40:49PM +0200, Tomas Härdin wrote:
> > > mån 2018-05-07 klockan 12:38 +0200 skrev Michael Niedermayer:
> > > > > Signed-off-by: Michael Niedermayer 
> > > > 
> > > > +if (sc->interlaced) {
> > > > +//Display F2 Offset
> > > > +mxf_write_local_tag(pb, 4, 0x3217);
> > > > +avio_wb32(pb, -((st->codecpar->height -
> > > > display_height)&1));
> > > 
> > > Negative values for DisplayF2Offset are not valid
> > 
> > The specification (SMPTE 377-1-2009) says:
> > 
> > The DisplayF2Offset Property adjusts the DisplayYOffset for the
> > second field relative to that for the first field. Its
> > value shall be zero (0) or minus 1. A value of minus 1 shall invert
> > the Displayed Topness relative to the Sampled
> > Topness.
> 
> Huh, the document I have is from 2004, which says "If the property is
> not present, its value shall be assumed to be 0. Valid values are zero
> or 1.". So I guess they changed that. I guess it's fine then.

will apply

thanks

[...]
-- 
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] [RFC][PATCH] configure: Disable unsafe demuxers by default

[wm4]

On Thu, 10 May 2018 16:44:59 +0100
Derek Buitenhuis  wrote:

> These demuxers have probes that mainly probe based on file extension,
> and map to codec IDs that render text as video. The result is that
> ffmpeg will, by default, happily render, for example, .txt files
> as images. This is not exactly a good security practice, an only
> makes it easier for potential attackers to gain the contents of
> system files.
> 
> Disable building these by default.
> 
> Signed-off-by: Derek Buitenhuis 
> ---

+1

You should send a patch that disables all those useless game demuxers
too. They only cause security issues and bloated library sizes.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Paul B Mahol]

On 5/11/18, wm4  wrote:
> On Thu, 10 May 2018 16:44:59 +0100
> Derek Buitenhuis  wrote:
>
>> These demuxers have probes that mainly probe based on file extension,
>> and map to codec IDs that render text as video. The result is that
>> ffmpeg will, by default, happily render, for example, .txt files
>> as images. This is not exactly a good security practice, an only
>> makes it easier for potential attackers to gain the contents of
>> system files.
>>
>> Disable building these by default.
>>
>> Signed-off-by: Derek Buitenhuis 
>> ---
>
> +1
>
> You should send a patch that disables all those useless game demuxers
> too. They only cause security issues and bloated library sizes.

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

Re: [FFmpeg-devel] [PATCH] flvenc: Fix sequence header update timestamps

[Alex Converse]

On Thu, May 3, 2018 at 10:58 AM, Jan Ekström  wrote:
>
> On Thu, May 3, 2018 at 7:50 PM, Alex Converse  wrote:
> > From: Alex Converse 
> >
> > ---
> >  libavformat/flvenc.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
> > index e8af48cb64..827d798a61 100644
> > --- a/libavformat/flvenc.c
> > +++ b/libavformat/flvenc.c
> > @@ -480,7 +480,7 @@ static int unsupported_codec(AVFormatContext *s,
> >  return AVERROR(ENOSYS);
> >  }
> >
> > -static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* 
> > par) {
> > +static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* 
> > par, unsigned int ts) {
>
> Hi,
>
> While the timestamp field is 8+24=32 bits, the DTS value is int64_ts.
> Thus, while I'm really bad at doing wrap-around logic, wouldn't it be
> something a la ´uint32_t wrapped_timestamp = (uint32_t)(ts %
> UINT32_MAX)`?
>

See the note about timestamps below.

> Additionally, I did briefly go through E.4.1 (FLV Tag) in the spec and
> it doesn't really seem to define whether this value is PTS or DTS...
> Is this defined anywhere proper? Given FLV's age I wouldn't be
> surprised that the answer would be "effectively DTS", of course. But
> if you've seen what  Adobe's implementation writes with B-frames it'd
> be interesting to see.
>

FLV AVC packets have a field called CompositionTime described to match
14496-12. FLV requires this composition offset to be zero for sequence header
type packets. This strongly indicates to me that we want DTS here. In addition
the current muxer and demuxer already use dts pretty consistently for this
value (and use cts = pts - dts).


> >  int64_t data_size;
> >  AVIOContext *pb = s->pb;
> >  FLVContext *flv = s->priv_data;
> > @@ -492,8 +492,8 @@ static void flv_write_codec_header(AVFormatContext* s, 
> > AVCodecParameters* par) {
> >  par->codec_type == AVMEDIA_TYPE_VIDEO ?
> >  FLV_TAG_TYPE_VIDEO : FLV_TAG_TYPE_AUDIO);
> >  avio_wb24(pb, 0); // size patched later
> > -avio_wb24(pb, 0); // ts
> > -avio_w8(pb, 0);   // ts ext
> > +avio_wb24(pb, ts & 0xFF);
> > +avio_w8(pb, (ts >> 24) & 0x7F);
>
> Looking at the spec this is bottom 24 bits of the timestamp and top 8,
> thus why is the second one not & 0xFF ?
>

The mapping from dts to the flv timestamp we write to the stream is identical
for to what we do for non header packets. [1] I can pull this into a separate
function but it's only two lines so it didn't seem worth while.

> >  avio_wb24(pb, 0); // streamid
> >  pos = avio_tell(pb);
> >  if (par->codec_id == AV_CODEC_ID_AAC) {
> > @@ -756,7 +756,7 @@ static int flv_write_header(AVFormatContext *s)
> >  }
> >
> >  for (i = 0; i < s->nb_streams; i++) {
> > -flv_write_codec_header(s, s->streams[i]->codecpar);
> > +flv_write_codec_header(s, s->streams[i]->codecpar, 0);
> >  }
> >
>
> Is it customary that even if you've got a live stream going, that the
> start point of a given ingest is always zero? In that case you might
> want to take mention of the initial dts, and then write offsets
> compared to that? Otherwise if you have an initial offset of, say, 5s,
> and then you a 5s GOP with an update, then while the difference to the
> initial timestamp would be 5s, using the original dts field you would
> get the second header having a value of 10s there. Or am I
> misunderstanding something here?

The FLV file (on disk) format requires the initial DTS to be zero. For FLV
over RTMP this is not the case.

At one point the flv muxer handled that itself, but that changed to handle
copyts better.[2]

For non-zero starting DTS should header packets have non-zero
timestamp? Maybe but that's the case irrespective of weather or
not we offset late extradata. Meanwhile many popular demuxers
(avformat included) special case initial extradata.

[1] 
https://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavformat/flvenc.c;h=e8af48cb6415a0e5795d7e36d82789ae1699f89f#l981
[2] 
https://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a0174f67298ba9494c146183dd360e637b03db64

>
> Otherwise, looks like a nice improvement in the implementation.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] flvenc: Fix sequence header update timestamps

[Alex Converse]

On Fri, May 4, 2018 at 7:11 AM, Josh de Kock  wrote:
>
>> On 4 May 2018, at 12:26, Michael Niedermayer  wrote:
>>
>>> On Fri, May 04, 2018 at 11:03:20AM +0800, Steven Liu wrote:
>>>
>>>
 On 4 May 2018, at 02:00, Jan Ekström  wrote:

 On Thu, May 3, 2018 at 8:58 PM, Jan Ekström  wrote:
> On Thu, May 3, 2018 at 7:50 PM, Alex Converse  
> wrote:
>> [...]
>>
>>> Will apply.
>>
>> I think Alex Converse has a git write account, so he can push himself if you
>> prefer
>>
>> thx
>>
>> [...]
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> In a rich man's house there is no place to spit but his face.
>> -- Diogenes of Sinope
>
> Hold off on pushing this for a day or two, I think Jan had some extra 
> comments (he said he was unable to reply until later as he is busy right now).
>

Sorry about the lateness on my part. Were there any later comments. I
haven't seen any. (And yes I can push this myself when all the
concerns are resolved).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

> You web people already have options for the various annoying whitelists.
>  Is this not covered by one of them?

As noted in my other reply to Paul, I find it a poor choice to shunt
the responsibility
of good/secure defaults to the user.

As a side note, derisively referring to me as "you web people" isn't
particularily charming.

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

Re: [FFmpeg-devel] [PATCH 6/6] fate/cbs: Add an SEI test

[Mark Thompson]

On 09/05/18 23:51, James Almer wrote:
> On 5/7/2018 9:30 PM, Mark Thompson wrote:
>> On 08/05/18 01:06, James Almer wrote:
>>> On 5/7/2018 8:11 PM, Mark Thompson wrote:
 The artificial sample file sei-1.h264 contains five frames (IDR P B I B)
 and the following SEI message types:
 * Buffering period
 * Picture timing
 * Pan-scan rectangle (display as 4:3)
 * User data registered, containing A/53 closed captions (captions match
   frame content, including reordering)
 * Recovery point (at the I frame)
 * Display orientation (identity transformation)
 * Mastering display (with arbitrary contents)
 * Undefined SEI type 1234 (containing ascending bytes)
 ---
 Sample file at .  (Named -1 
 anticipating the possibility of adding more in future.)

 140ddba635960ac73935a0fb268748a43c647151  fate/h264/sei-1.h264
>>>
>>> If you crafted this file using the mastering display implementation from
>>> patch 5/6 then you may need to create it again with the fixed element sizes.
>>
>> It didn't change because the numbers I had there were < 2^16, but I've 
>> regenerated anyway with a larger number as max so that it does get tested.
>>
>> b8f3b5443c8e437a83d36a72657478529c2afcfc  fate/h264/sei-1.h264
> 
> Uploaded then.

Thank you!

Set applied.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

> Disabling demuxers by default does not seem to be a good idea to me.

So rendering arbitrary text files by default seems like a good idea in
comparsion?

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Paul B Mahol]

On 5/11/18, Derek Buitenhuis  wrote:
>> Disabling demuxers by default does not seem to be a good idea to me.
>
> So rendering arbitrary text files by default seems like a good idea in
> comparsion?

That is useful feature.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] flvenc: Fix sequence header update timestamps

[Jan Ekström]

On Fri, May 11, 2018 at 1:39 AM, Alex Converse  wrote:
> On Thu, May 3, 2018 at 10:58 AM, Jan Ekström  wrote:
>>
>> Hi,
>>
>> While the timestamp field is 8+24=32 bits, the DTS value is int64_ts.
>> Thus, while I'm really bad at doing wrap-around logic, wouldn't it be
>> something a la ´uint32_t wrapped_timestamp = (uint32_t)(ts %
>> UINT32_MAX)`?
>>
>
> See the note about timestamps below.
>
>> Additionally, I did briefly go through E.4.1 (FLV Tag) in the spec and
>> it doesn't really seem to define whether this value is PTS or DTS...
>> Is this defined anywhere proper? Given FLV's age I wouldn't be
>> surprised that the answer would be "effectively DTS", of course. But
>> if you've seen what  Adobe's implementation writes with B-frames it'd
>> be interesting to see.
>>
>
> FLV AVC packets have a field called CompositionTime described to match
> 14496-12. FLV requires this composition offset to be zero for sequence header
> type packets. This strongly indicates to me that we want DTS here. In addition
> the current muxer and demuxer already use dts pretty consistently for this
> value (and use cts = pts - dts).
>

Alright, so dts it is... That only leaves out the fact that we're
stuffing int64_t into an "unsigned int", which most likely will not
warn on 64bit linux, but most likely will on various architectures
and/or systems.

So we:
1) take in the dts as int64_t.
2) apply wrap-over at either 32 or 31 bits depending on if the signed
value is ever supposed to be negative. Depends on how wrap-around in
FLV is supposed to be handled (aka "Is the negative area ever supposed
to be used from the 32bit signed integer field?").
3) Write it there.

>
>> >  int64_t data_size;
>> >  AVIOContext *pb = s->pb;
>> >  FLVContext *flv = s->priv_data;
>> > @@ -492,8 +492,8 @@ static void flv_write_codec_header(AVFormatContext* s, 
>> > AVCodecParameters* par) {
>> >  par->codec_type == AVMEDIA_TYPE_VIDEO ?
>> >  FLV_TAG_TYPE_VIDEO : FLV_TAG_TYPE_AUDIO);
>> >  avio_wb24(pb, 0); // size patched later
>> > -avio_wb24(pb, 0); // ts
>> > -avio_w8(pb, 0);   // ts ext
>> > +avio_wb24(pb, ts & 0xFF);
>> > +avio_w8(pb, (ts >> 24) & 0x7F);
>>
>> Looking at the spec this is bottom 24 bits of the timestamp and top 8,
>> thus why is the second one not & 0xFF ?
>>
>
> The mapping from dts to the flv timestamp we write to the stream is identical
> for to what we do for non header packets. [1] I can pull this into a separate
> function but it's only two lines so it didn't seem worth while.

Yes, I think I commented on this that I hadn't noticed the signedness
of the value there :) So this is 100% a-OK.

>
>> >  avio_wb24(pb, 0); // streamid
>> >  pos = avio_tell(pb);
>> >  if (par->codec_id == AV_CODEC_ID_AAC) {
>> > @@ -756,7 +756,7 @@ static int flv_write_header(AVFormatContext *s)
>> >  }
>> >
>> >  for (i = 0; i < s->nb_streams; i++) {
>> > -flv_write_codec_header(s, s->streams[i]->codecpar);
>> > +flv_write_codec_header(s, s->streams[i]->codecpar, 0);
>> >  }
>> >
>>
>> Is it customary that even if you've got a live stream going, that the
>> start point of a given ingest is always zero? In that case you might
>> want to take mention of the initial dts, and then write offsets
>> compared to that? Otherwise if you have an initial offset of, say, 5s,
>> and then you a 5s GOP with an update, then while the difference to the
>> initial timestamp would be 5s, using the original dts field you would
>> get the second header having a value of 10s there. Or am I
>> misunderstanding something here?
>
> The FLV file (on disk) format requires the initial DTS to be zero. For FLV
> over RTMP this is not the case.
>

Oh, interesting... makes sense, of course, for continuing a stream
relative to the last timestamp of the previous ingest.

> At one point the flv muxer handled that itself, but that changed to handle
> copyts better.[2]
>
> For non-zero starting DTS should header packets have non-zero
> timestamp? Maybe but that's the case irrespective of weather or
> not we offset late extradata. Meanwhile many popular demuxers
> (avformat included) special case initial extradata.

Interesting, so in theory we should be making the initial
initialization data to be according to the DTS according to your
reading? Or is the initial extradata packet always supposed to be
zero, irrelevant of the actual initial content DTS?

As soon as these questions can be answered I think we've got the ball
rolling on getting this in. As I mentioned already, this is an
improvement.


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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

> please correct me if iam wrong, theres quite a bit iam guessing here
> IIUC the problem is that in your usecase
> 1. ffmpeg has access to sensitive files
> 2. one of these files can be opened by an attacker with ffmpeg
> 2b. This involves the file being probed as a supported format

It is "probed" as file extension mostly. .txt is one of these
extensions, which 99.999%
of the time, is not a multimedia file, for example.

> 3. The file is then demuxed, decoded, encoded, muxed
> 4. the result is given back to the attacker
>
> This patchset removes one of the demuxers involved in the attack

Not removed. Disabled. As discussed in other replies, making it manual
also seems
fine to me. Rendering arbitrary text files as images (as in, rendering
the text using
a built in font, to an image with that text in it) isn't exactly a
great and secure default
behavior, especially for the world's most commont text file extension...

> The first problem of this patchset is that it does not provide any
> evidence of how the other demuxers probe functions can trigger on
> random text files.

ffmpeg -i something.txt a.png

But really, it is not necssarily an attack on its own (it could be),
but it makes any
other attack vastly easier to exploit. For example, that HLS stuff
avformat had fixed
last year could have pointed at various .txt files. I don't really
understand why the
concept of "rendering arbitrary text files as images" is not obviously bad?

> for example idf_probe() requires, the first 12 bytes of the file  to
> match exactly and some of these are not text. So a attack which depends
> on the probing of sensitive text data to succeed should not work with it

it is not specifci to probing. It's choosing e.g. the ansi decoder based off the
.txt file extension, for example. Or the bintext decoder based off the
.bin extension.

> The second problem i see is that the patch disables the demuxers, while
> disabling the probe functions affected should have the same effect
> security wise but is a smaller step in respect to disabling.

As noted in my reply to Marton and Gyan, I'm fine with this approach,
as the implications
are the same.

> The third problem i see is that really once you read sensitive data
> and pass something back to the user you already lost.

[...]

> A text demuxer and decoder makes it easier and it makes it much easier
> to demonstrate the attack in a flashy way. But having the probe code
> access the sensitive data even if none succeeds already makes quite
> a bit of internal state (caches, branch prediction, deallocated memory, ...)
> be contaminated with sensitive information. Its hard to ensure that
> none of this can be recovered by the attacker.

Ah the classic "well it can technically be beaten, you may as well make it
as easy as possible" argument that people use to argue against things
like ASLR... :|

The point is to minimize the risk where possible, with good defaults for *all*
users. It's not a great thing to just point at users and go "well you
should have
done a better job containerizing/sandboxing/etc." IMO.

> I think first ffmpeg should not be able to access sensitive data.
> Then none of our probe functions should succeed on the average
> sensitive text file. If one does, we should look into making it not
> detect that.

The succeed because libavformat loves probing based on file extenions.

> Also it may seem counter-intuitive but adding a probe function which
> is designed to succeed on sensitive text files may be more reliable
> to stop their detection than to disable probe functions.

This is just adding more heuristics to probing, though

> The probe system is basically doing differential probing. That is
> the one of 2 that has the higher score wins.
> So it may be more effective to add a function that that returns a
> high score intentionally for a null demuxer than to try to stop
> every function that returns more than 0

Wouldn't this break a lot of currently "working" detection of
bad/broken files that
users seem to rely on? (I don't think they should rely on it, but
that's just my opinion.)

> if such probe code is added, it also could maybe warn the admin about a
> potential misconfiguration that allows probing to reach to sensitive
> data.

[...]

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

Re: [FFmpeg-devel] [PATCH] flvenc: Fix sequence header update timestamps

[Jan Ekström]

On Fri, May 11, 2018 at 1:48 AM, Alex Converse  wrote:
>
> Sorry about the lateness on my part. Were there any later comments. I
> haven't seen any. (And yes I can push this myself when all the
> concerns are resolved).

It's OK, it has been a rather busy time for me as well.

I was already thinking if you had given up on this and if I should
just apply the changes I thought were correct and re-post it for
review myself:
* making the function take in int64_t.
* DTS is most likely what's wanted so no change there.
* most likely FLV never used negative timestamps (even though the
value is signed) so:
  1.wrap-around at the 31 bits for the signed integer value, and then
  2. keep the 0x7F because the topmost bit of a signed field would
never be used.
* try to figure out if the initial metadata packet should specifically
be starting with a zero timestamp, or the initial DTS of the input?

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Rostislav Pehlivanov]

On 10 May 2018 at 23:27, Paul B Mahol  wrote:

> On 5/11/18, wm4  wrote:
> > On Thu, 10 May 2018 16:44:59 +0100
> > Derek Buitenhuis  wrote:
> >
> >> These demuxers have probes that mainly probe based on file extension,
> >> and map to codec IDs that render text as video. The result is that
> >> ffmpeg will, by default, happily render, for example, .txt files
> >> as images. This is not exactly a good security practice, an only
> >> makes it easier for potential attackers to gain the contents of
> >> system files.
> >>
> >> Disable building these by default.
> >>
> >> Signed-off-by: Derek Buitenhuis 
> >> ---
> >
> > +1
> >
> > You should send a patch that disables all those useless game demuxers
> > too. They only cause security issues and bloated library sizes.
>
> Against.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

I agree with Paul, game demuxers are useful, don't bloat much and can be
fixed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Paul B Mahol]

On 5/11/18, Derek Buitenhuis  wrote:
>> please correct me if iam wrong, theres quite a bit iam guessing here
>> IIUC the problem is that in your usecase
>> 1. ffmpeg has access to sensitive files
>> 2. one of these files can be opened by an attacker with ffmpeg
>> 2b. This involves the file being probed as a supported format
>
> It is "probed" as file extension mostly. .txt is one of these
> extensions, which 99.999%
> of the time, is not a multimedia file, for example.
>
>> 3. The file is then demuxed, decoded, encoded, muxed
>> 4. the result is given back to the attacker
>>
>> This patchset removes one of the demuxers involved in the attack
>
> Not removed. Disabled. As discussed in other replies, making it manual
> also seems
> fine to me. Rendering arbitrary text files as images (as in, rendering
> the text using
> a built in font, to an image with that text in it) isn't exactly a
> great and secure default
> behavior, especially for the world's most commont text file extension...
>
>> The first problem of this patchset is that it does not provide any
>> evidence of how the other demuxers probe functions can trigger on
>> random text files.
>
> ffmpeg -i something.txt a.png
>
> But really, it is not necssarily an attack on its own (it could be),
> but it makes any
> other attack vastly easier to exploit. For example, that HLS stuff
> avformat had fixed
> last year could have pointed at various .txt files. I don't really
> understand why the
> concept of "rendering arbitrary text files as images" is not obviously bad?
>
>> for example idf_probe() requires, the first 12 bytes of the file  to
>> match exactly and some of these are not text. So a attack which depends
>> on the probing of sensitive text data to succeed should not work with it
>
> it is not specifci to probing. It's choosing e.g. the ansi decoder based off
> the
> .txt file extension, for example. Or the bintext decoder based off the
> .bin extension.
>
>> The second problem i see is that the patch disables the demuxers, while
>> disabling the probe functions affected should have the same effect
>> security wise but is a smaller step in respect to disabling.
>
> As noted in my reply to Marton and Gyan, I'm fine with this approach,
> as the implications
> are the same.
>
>> The third problem i see is that really once you read sensitive data
>> and pass something back to the user you already lost.
>
> [...]
>
>> A text demuxer and decoder makes it easier and it makes it much easier
>> to demonstrate the attack in a flashy way. But having the probe code
>> access the sensitive data even if none succeeds already makes quite
>> a bit of internal state (caches, branch prediction, deallocated memory,
>> ...)
>> be contaminated with sensitive information. Its hard to ensure that
>> none of this can be recovered by the attacker.
>
> Ah the classic "well it can technically be beaten, you may as well make it
> as easy as possible" argument that people use to argue against things
> like ASLR... :|
>
> The point is to minimize the risk where possible, with good defaults for
> *all*
> users. It's not a great thing to just point at users and go "well you
> should have
> done a better job containerizing/sandboxing/etc." IMO.
>
>> I think first ffmpeg should not be able to access sensitive data.
>> Then none of our probe functions should succeed on the average
>> sensitive text file. If one does, we should look into making it not
>> detect that.
>
> The succeed because libavformat loves probing based on file extenions.
>
>> Also it may seem counter-intuitive but adding a probe function which
>> is designed to succeed on sensitive text files may be more reliable
>> to stop their detection than to disable probe functions.
>
> This is just adding more heuristics to probing, though
>
>> The probe system is basically doing differential probing. That is
>> the one of 2 that has the higher score wins.
>> So it may be more effective to add a function that that returns a
>> high score intentionally for a null demuxer than to try to stop
>> every function that returns more than 0
>
> Wouldn't this break a lot of currently "working" detection of
> bad/broken files that
> users seem to rely on? (I don't think they should rely on it, but
> that's just my opinion.)
>
>> if such probe code is added, it also could maybe warn the admin about a
>> potential misconfiguration that allows probing to reach to sensitive
>> data.
>
> [...]

If you think you are fixing security issue you are very wrong.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

> If you think you are fixing security issue you are very wrong.

You've nailed that "saying what you think" part of communication, but
need to word a little
on the all important "saying why you think that" part. Keep
practicing, you can do it. I
believe in you.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Rostislav Pehlivanov]

On 11 May 2018 at 00:04, Paul B Mahol  wrote:

> On 5/11/18, Derek Buitenhuis  wrote:
> >> Disabling demuxers by default does not seem to be a good idea to me.
> >
> > So rendering arbitrary text files by default seems like a good idea in
> > comparsion?
>
> That is useful feature.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

No, it isn't. Its not useful at all, you have no control over how they're
rendered and they're always rendered using CGA fonts from the 80's. They're
also very easily misprobed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Paul B Mahol]

On 5/11/18, Derek Buitenhuis  wrote:
>> If you think you are fixing security issue you are very wrong.
>
> You've nailed that "saying what you think" part of communication, but
> need to word a little
> on the all important "saying why you think that" part. Keep
> practicing, you can do it. I
> believe in you.

I do not have time to explain security basics.
Get a decent book and read it from a start to an end.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[wm4]

On Fri, 11 May 2018 00:21:37 +0100
Rostislav Pehlivanov  wrote:

> On 10 May 2018 at 23:27, Paul B Mahol  wrote:
> 
> > On 5/11/18, wm4  wrote:  
> > > On Thu, 10 May 2018 16:44:59 +0100
> > > Derek Buitenhuis  wrote:
> > >  
> > >> These demuxers have probes that mainly probe based on file extension,
> > >> and map to codec IDs that render text as video. The result is that
> > >> ffmpeg will, by default, happily render, for example, .txt files
> > >> as images. This is not exactly a good security practice, an only
> > >> makes it easier for potential attackers to gain the contents of
> > >> system files.
> > >>
> > >> Disable building these by default.
> > >>
> > >> Signed-off-by: Derek Buitenhuis 
> > >> ---  
> > >
> > > +1
> > >
> > > You should send a patch that disables all those useless game demuxers
> > > too. They only cause security issues and bloated library sizes.  
> >
> > Against.
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >  
> 
> I agree with Paul, game demuxers are useful, don't bloat much and can be
> fixed.

Experience shows that it's always the obscure features which cause
security issues. Regarding the bloat: these small things add up a lot,
and suddenly you have hundreds of demuxers. It's hard to filter them
out manually, and why make each user do that? Many of these game formats
in particular probably have something like under a dozen files in the
universe that exist at all (such as the files included in a particular
game release).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Fri, May 11, 2018 at 12:35 AM, Paul B Mahol  wrote:
> I do not have time to explain security basics.
> Get a decent book and read it from a start to an end.

Speaking frankly for a second: Why do people put up with this sort of
crud on this
mailing list?

Insult-laden fact-less response. It's insane. It's like I'm flashing
back to ffmpeg-devel
in 2006.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Fri, May 11, 2018 at 12:41 AM, Derek Buitenhuis
 wrote:
> Speaking frankly for a second: Why do people put up with this sort of
> crud on this
> mailing list?

Unrelatedly, sorry for the broken linebreaks. Bad MUA...

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[wm4]

On Fri, 11 May 2018 00:41:20 +0100
Derek Buitenhuis  wrote:

> On Fri, May 11, 2018 at 12:35 AM, Paul B Mahol  wrote:
> > I do not have time to explain security basics.
> > Get a decent book and read it from a start to an end.  
> 
> Speaking frankly for a second: Why do people put up with this sort of
> crud on this
> mailing list?
> 
> Insult-laden fact-less response. It's insane. It's like I'm flashing
> back to ffmpeg-devel
> in 2006.

Because there are no consequences, no leadership, no conflict
arbitration. It's a joke of a project.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Fri, May 11, 2018 at 12:36 AM, wm4  wrote:
> Experience shows that it's always the obscure features which cause
> security issues. Regarding the bloat: these small things add up a lot,
> and suddenly you have hundreds of demuxers. It's hard to filter them
> out manually, and why make each user do that? Many of these game formats
> in particular probably have something like under a dozen files in the
> universe that exist at all (such as the files included in a particular
> game release).

I think this is a tangental battle for another day. Not really
related to the patch/RFC at hand.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[James Darnley]

On 2018-05-11 00:57, Derek Buitenhuis wrote:
>> Disabling demuxers by default does not seem to be a good idea to me.
> 
> So rendering arbitrary text files by default seems like a good idea in
> comparsion?

I want to argue some more so here you go: it isn't "by default".

It gets rendered because you asked for it to be rendered.  You asked for
/etc/passwd to be rendered so ffmpeg did that.  It produced a nice 4K
video of the file with all your secrets clearly legible in it.  Why do
you care?  Surely nobody will see it.  Surely you're not going to upload
this file to the public Internet.

I don't care that you do encode any random file that someone uploads to
you.  I don't care that you do put the results on the public net.  I do
care a little that ffmpeg understands playlist files but not in the same
way you do.  I do care a little that ffmpeg does so much magic for you
but not in the same way you do.

I haven't tried to stand in the way of other bad changes to ffmpeg (like
the fact that the flac muxer will now mux video streams) and I won't try
to stand in the way of this one.

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mpegtsenc: set AVFMT_NODIMENSIONS

[Michael Niedermayer]

On Wed, May 09, 2018 at 07:23:35PM -0700, Aman Gupta wrote:
> From: Aman Gupta 
> 
> This allows remuxing streams from one mpegts container to another,
> without requiring avformat_find_stream_info() (or using `ffmpeg
> -probesize 32` on the cli).
> ---
>  libavformat/mpegtsenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Rostislav Pehlivanov]

On 11 May 2018 at 00:44, wm4  wrote:

> On Fri, 11 May 2018 00:41:20 +0100
> Derek Buitenhuis  wrote:
>
> > On Fri, May 11, 2018 at 12:35 AM, Paul B Mahol  wrote:
> > > I do not have time to explain security basics.
> > > Get a decent book and read it from a start to an end.
> >
> > Speaking frankly for a second: Why do people put up with this sort of
> > crud on this
> > mailing list?
> >
> > Insult-laden fact-less response. It's insane. It's like I'm flashing
> > back to ffmpeg-devel
> > in 2006.
>
> Because there are no consequences


There are consequeces, the consequences being broken design and/or features


no leadership


Just because there are many people determining what happens doesn't mean no
one is leading.


no conflict arbitration.
>

There is conflict arbitration, we're doing it right now in fact.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[wm4]

On Fri, 11 May 2018 00:53:16 +0100
Rostislav Pehlivanov  wrote:

> On 11 May 2018 at 00:44, wm4  wrote:
> 
> > On Fri, 11 May 2018 00:41:20 +0100
> > Derek Buitenhuis  wrote:
> >  
> > > On Fri, May 11, 2018 at 12:35 AM, Paul B Mahol  wrote:  
> > > > I do not have time to explain security basics.
> > > > Get a decent book and read it from a start to an end.  
> > >
> > > Speaking frankly for a second: Why do people put up with this sort of
> > > crud on this
> > > mailing list?
> > >
> > > Insult-laden fact-less response. It's insane. It's like I'm flashing
> > > back to ffmpeg-devel
> > > in 2006.  
> >
> > Because there are no consequences  
> 
> 
> There are consequeces, the consequences being broken design and/or features
> 
> 
> no leadership
> 
> 
> Just because there are many people determining what happens doesn't mean no
> one is leading.
> 
> 
> no conflict arbitration.
> >  
> 
> There is conflict arbitration, we're doing it right now in fact.

Yeah, we can just flame each other as hard as we can every time, until
someone gives up out of boredom or disgust. This is why ffmpeg is one
of the worst open source projects around.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[Derek Buitenhuis]

On Fri, May 11, 2018 at 12:49 AM, James Darnley  wrote:
> I want to argue some more so here you go: it isn't "by default".

Strange definition of default, but OK.

> It gets rendered because you asked for it to be rendered.  You asked for
> /etc/passwd to be rendered so ffmpeg did that.  It produced a nice 4K
> video of the file with all your secrets clearly legible in it.  Why do
> you care?  Surely nobody will see it.  Surely you're not going to upload
> this file to the public Internet.

I think you might lack some imagination on how people use the API,
nd how people use the FFmpeg cli. This for example, pre-HLS CVE patches,
let someone upload a m3u8 that pointed to e.g. /something/password.txt
and have it rendered. It's not a all-or-nothing sort of thing.

> I don't care that you do encode any random file that someone uploads to
> you.  I don't care that you do put the results on the public net.  I do
> care a little that ffmpeg understands playlist files but not in the same
> way you do.  I do care a little that ffmpeg does so much magic for you
> but not in the same way you do.

So basically you don't care at all about making ffmpeg less exploitable by
various users. Great.

Gotta say, this is one mighty poor attitude towards security. But OK.

> I haven't tried to stand in the way of other bad changes to ffmpeg (like
> the fact that the flac muxer will now mux video streams) and I won't try
> to stand in the way of this one.

Fine. Patch dropped. Have a good day/night. I don't have the mental
energy to deal with this endless infighting, feces-flinging, and insane
approach to safety/sanity. I'm not going to spend my spare time
smashing my head against a wall and being insulted.

This will be my last response in the thread.

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

Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default

[wm4]

On Fri, 11 May 2018 01:49:58 +0200
James Darnley  wrote:

> ...

Security in ffmpeg sure is weird. On one hand we get all kinds of crazy
stuff that's supposed to mitigate... something (like whitelists), on
the other hand we get this.

> I haven't tried to stand in the way of other bad changes to ffmpeg (like
> the fact that the flac muxer will now mux video streams) and I won't try
> to stand in the way of this one.

That sounds insane, but it's probably about cover art. Only ffmpeg is
insane enough to treat cover art as single-frame video stream. (And
as an API user I've really tried hard to make this work, but it remains
a painful special case.)

So, not sure why you claim it's a "bad change" to add support for that
in the flac muxer. It merely follows the ffmpeg API.

Anyway, I guess D. B. is right and this thread is a fucking waste of
time, so goodbye.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3 1/4] cbs_h265: read/write HEVC PREFIX SEI

[Xiang, Haihao]

On Thu, 2018-05-10 at 21:00 +0100, Mark Thompson wrote:
> On 10/05/18 06:47, Haihao Xiang wrote:
> > Similar to H264, cbs_h265_{read, write}_nal_unit() can handle HEVC
> > prefix SEI NAL units. Currently mastering display colour volume SEI
> > message is added only, we may add more SEI message if needed later
> > 
> > v2: Fix coding style and rebase the code
> > 
> > v3: Fix assertion failure when running the command below:
> > ffmpeg -i input-with-sei.mkv -c:v copy -bsf:v trace_headers -f null -
> > Fix the ranges for max_display_mastering_luminance and
> > min_display_mastering_luminance
> > 
> > Signed-off-by: Haihao Xiang 
> > ---
> >  libavcodec/cbs_h2645.c|  45 ++
> >  libavcodec/cbs_h265.h |  36 
> >  libavcodec/cbs_h265_syntax_template.c | 158
> > ++
> >  3 files changed, 239 insertions(+)
> > 
> > ...
> >  
> > +typedef struct H265RawSEIMasteringDiplayColourVolume {
> 
> I fixed this typo ^^ which appears in several places (a
> bad feature of code completion, I guess :P ).
> 
> Then tested, all looks good, applied.

Thanks for catching and fixing this stupid typo, I should check spelling more
carefully. 

> 
> Thanks!
> 
> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] hwcontext_vaapi: Add an assert in vaapi_map_from_drm()

[Xiang, Haihao]

On Thu, 2018-05-10 at 20:04 +0100, Mark Thompson wrote:
> On 10/05/18 04:08, Haihao Xiang wrote:
> > The flag of input_available must be set when pic_start is not NULL, so
> > add an assert to ensure it is true. In addition, the assert on last_pic
> > is unnecessary now, so remove this assert.
> > 
> > Signed-off-by: Haihao Xiang 
> > ---
> >  libavcodec/vaapi_encode.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> On 10/05/18 04:08, Haihao Xiang wrote:
> > Every fourcc in vaapi_drm_format_map should be in
> > vaapi_format_map, so add an assert to ensure we have the right maps.
> > 
> > Signed-off-by: Haihao Xiang 
> > ---
> >  libavutil/hwcontext_vaapi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Both applied.

Thanks for applying the patches.

Best Regards
Haihao

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

Re: [FFmpeg-devel] [PATCH] flvenc: Fix sequence header update timestamps

[Alex Converse]

On Thu, May 10, 2018 at 4:08 PM, Jan Ekström  wrote:
> On Fri, May 11, 2018 at 1:39 AM, Alex Converse  
> wrote:
>> On Thu, May 3, 2018 at 10:58 AM, Jan Ekström  wrote:
>>>
>>> Hi,
>>>
>>> While the timestamp field is 8+24=32 bits, the DTS value is int64_ts.
>>> Thus, while I'm really bad at doing wrap-around logic, wouldn't it be
>>> something a la ´uint32_t wrapped_timestamp = (uint32_t)(ts %
>>> UINT32_MAX)`?
>>>
>>
>> See the note about timestamps below.
>>
>>> Additionally, I did briefly go through E.4.1 (FLV Tag) in the spec and
>>> it doesn't really seem to define whether this value is PTS or DTS...
>>> Is this defined anywhere proper? Given FLV's age I wouldn't be
>>> surprised that the answer would be "effectively DTS", of course. But
>>> if you've seen what  Adobe's implementation writes with B-frames it'd
>>> be interesting to see.
>>>
>>
>> FLV AVC packets have a field called CompositionTime described to match
>> 14496-12. FLV requires this composition offset to be zero for sequence header
>> type packets. This strongly indicates to me that we want DTS here. In 
>> addition
>> the current muxer and demuxer already use dts pretty consistently for this
>> value (and use cts = pts - dts).
>>
>
> Alright, so dts it is... That only leaves out the fact that we're
> stuffing int64_t into an "unsigned int", which most likely will not
> warn on 64bit linux, but most likely will on various architectures
> and/or systems.
>
> So we:
> 1) take in the dts as int64_t.
> 2) apply wrap-over at either 32 or 31 bits depending on if the signed
> value is ever supposed to be negative. Depends on how wrap-around in
> FLV is supposed to be handled (aka "Is the negative area ever supposed
> to be used from the 32bit signed integer field?").
> 3) Write it there.
>
>>
>>> >  int64_t data_size;
>>> >  AVIOContext *pb = s->pb;
>>> >  FLVContext *flv = s->priv_data;
>>> > @@ -492,8 +492,8 @@ static void flv_write_codec_header(AVFormatContext* 
>>> > s, AVCodecParameters* par) {
>>> >  par->codec_type == AVMEDIA_TYPE_VIDEO ?
>>> >  FLV_TAG_TYPE_VIDEO : FLV_TAG_TYPE_AUDIO);
>>> >  avio_wb24(pb, 0); // size patched later
>>> > -avio_wb24(pb, 0); // ts
>>> > -avio_w8(pb, 0);   // ts ext
>>> > +avio_wb24(pb, ts & 0xFF);
>>> > +avio_w8(pb, (ts >> 24) & 0x7F);
>>>
>>> Looking at the spec this is bottom 24 bits of the timestamp and top 8,
>>> thus why is the second one not & 0xFF ?
>>>
>>
>> The mapping from dts to the flv timestamp we write to the stream is identical
>> for to what we do for non header packets. [1] I can pull this into a separate
>> function but it's only two lines so it didn't seem worth while.
>
> Yes, I think I commented on this that I hadn't noticed the signedness
> of the value there :) So this is 100% a-OK.
>
>>
>>> >  avio_wb24(pb, 0); // streamid
>>> >  pos = avio_tell(pb);
>>> >  if (par->codec_id == AV_CODEC_ID_AAC) {
>>> > @@ -756,7 +756,7 @@ static int flv_write_header(AVFormatContext *s)
>>> >  }
>>> >
>>> >  for (i = 0; i < s->nb_streams; i++) {
>>> > -flv_write_codec_header(s, s->streams[i]->codecpar);
>>> > +flv_write_codec_header(s, s->streams[i]->codecpar, 0);
>>> >  }
>>> >
>>>
>>> Is it customary that even if you've got a live stream going, that the
>>> start point of a given ingest is always zero? In that case you might
>>> want to take mention of the initial dts, and then write offsets
>>> compared to that? Otherwise if you have an initial offset of, say, 5s,
>>> and then you a 5s GOP with an update, then while the difference to the
>>> initial timestamp would be 5s, using the original dts field you would
>>> get the second header having a value of 10s there. Or am I
>>> misunderstanding something here?
>>
>> The FLV file (on disk) format requires the initial DTS to be zero. For FLV
>> over RTMP this is not the case.
>>
>
> Oh, interesting... makes sense, of course, for continuing a stream
> relative to the last timestamp of the previous ingest.
>
>> At one point the flv muxer handled that itself, but that changed to handle
>> copyts better.[2]
>>
>> For non-zero starting DTS should header packets have non-zero
>> timestamp? Maybe but that's the case irrespective of weather or
>> not we offset late extradata. Meanwhile many popular demuxers
>> (avformat included) special case initial extradata.
>
> Interesting, so in theory we should be making the initial
> initialization data to be according to the DTS according to your
> reading? Or is the initial extradata packet always supposed to be
> zero, irrelevant of the actual initial content DTS?
>
> As soon as these questions can be answered I think we've got the ball
> rolling on getting this in. As I mentioned already, this is an
> improvement.
>

There are some subtle differences in how FLV timestamps are handled on the
wire in rtmp vs in files, and I think that's what's creating most of
he comple

[FFmpeg-devel] [PATCH 1/2] flvenc: Factorize timestamp writing

[Alex Converse]

From: Alex Converse 

The code is trivial but the semantics in the spec are ambiguous. This
should help keep parts of the muxer interpreting them consistently.
---
 libavformat/flvenc.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
index e8af48cb64..9b7cdfe7db 100644
--- a/libavformat/flvenc.c
+++ b/libavformat/flvenc.c
@@ -230,12 +230,17 @@ static void put_amf_string(AVIOContext *pb, const char 
*str)
 avio_write(pb, str, len);
 }
 
+// FLV timestamps are 32 bits signed, RTMP timestamps should be 32-bit unsigned
+static void put_timestamp(AVIOContext *pb, unsigned ts) {
+avio_wb24(pb, ts & 0xFF);
+avio_w8(pb, (ts >> 24) & 0x7F);
+}
+
 static void put_avc_eos_tag(AVIOContext *pb, unsigned ts)
 {
 avio_w8(pb, FLV_TAG_TYPE_VIDEO);
 avio_wb24(pb, 5);   /* Tag Data Size */
-avio_wb24(pb, ts);  /* lower 24 bits of timestamp in ms */
-avio_w8(pb, (ts >> 24) & 0x7F); /* MSB of ts in ms */
+put_timestamp(pb, ts);
 avio_wb24(pb, 0);   /* StreamId = 0 */
 avio_w8(pb, 23);/* ub[4] FrameType = 1, ub[4] CodecId = 7 
*/
 avio_w8(pb, 2); /* AVC end of sequence */
@@ -978,8 +983,7 @@ static int flv_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 }
 
 avio_wb24(pb, size + flags_size);
-avio_wb24(pb, ts & 0xFF);
-avio_w8(pb, (ts >> 24) & 0x7F); // timestamps are 32 bits _signed_
+put_timestamp(pb, ts);
 avio_wb24(pb, flv->reserved);
 
 if (par->codec_type == AVMEDIA_TYPE_DATA ||
-- 
2.17.0

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

[FFmpeg-devel] [PATCH 2/2] flvenc: Fix sequence header update timestamps

[Alex Converse]

From: Alex Converse 

---
 libavformat/flvenc.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
index 9b7cdfe7db..7aa2dbf9a6 100644
--- a/libavformat/flvenc.c
+++ b/libavformat/flvenc.c
@@ -485,7 +485,7 @@ static int unsupported_codec(AVFormatContext *s,
 return AVERROR(ENOSYS);
 }
 
-static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par) 
{
+static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par, 
unsigned ts) {
 int64_t data_size;
 AVIOContext *pb = s->pb;
 FLVContext *flv = s->priv_data;
@@ -497,8 +497,7 @@ static void flv_write_codec_header(AVFormatContext* s, 
AVCodecParameters* par) {
 par->codec_type == AVMEDIA_TYPE_VIDEO ?
 FLV_TAG_TYPE_VIDEO : FLV_TAG_TYPE_AUDIO);
 avio_wb24(pb, 0); // size patched later
-avio_wb24(pb, 0); // ts
-avio_w8(pb, 0);   // ts ext
+put_timestamp(pb, ts);
 avio_wb24(pb, 0); // streamid
 pos = avio_tell(pb);
 if (par->codec_id == AV_CODEC_ID_AAC) {
@@ -761,7 +760,7 @@ static int flv_write_header(AVFormatContext *s)
 }
 
 for (i = 0; i < s->nb_streams; i++) {
-flv_write_codec_header(s, s->streams[i]->codecpar);
+flv_write_codec_header(s, s->streams[i]->codecpar, 0);
 }
 
 flv->datastart_offset = avio_tell(pb);
@@ -905,7 +904,7 @@ static int flv_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 }
 memcpy(par->extradata, side, side_size);
 par->extradata_size = side_size;
-flv_write_codec_header(s, par);
+flv_write_codec_header(s, par, (unsigned)pkt->dts);
 }
 }
 
-- 
2.17.0

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

[FFmpeg-devel] [PATCH 2/4] lavc/h2645_parse: rename the nal_unit_name to hevc_nal_unit_name.

[Jun Zhao]

Rename the nal_unit_name to hevc_nal_unit_name, will add a
h264_nal_unit_name function.

Signed-off-by: Jun Zhao 
---
 libavcodec/h2645_parse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index ec25c1e..579b2c9 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -145,7 +145,7 @@ nsc:
 return si;
 }
 
-static const char *nal_unit_name(int nal_type)
+static const char *hevc_nal_unit_name(int nal_type)
 {
 switch(nal_type) {
 case HEVC_NAL_TRAIL_N: return "TRAIL_N";
@@ -239,7 +239,7 @@ static int hevc_parse_nal_header(H2645NAL *nal, void 
*logctx)
 
 av_log(logctx, AV_LOG_DEBUG,
"nal_unit_type: %d(%s), nuh_layer_id: %d, temporal_id: %d\n",
-   nal->type, nal_unit_name(nal->type), nuh_layer_id, 
nal->temporal_id);
+   nal->type, hevc_nal_unit_name(nal->type), nuh_layer_id, 
nal->temporal_id);
 
 return nuh_layer_id == 0;
 }
-- 
2.7.4

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

[FFmpeg-devel] [PATCH 1/4] lavc/h2645_parse: log more HEVC NAL type.

[Jun Zhao]

Signed-off-by: Jun Zhao 
---
 libavcodec/h2645_parse.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index dbf2435..ec25c1e 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -158,12 +158,28 @@ static const char *nal_unit_name(int nal_type)
 case HEVC_NAL_RADL_R : return "RADL_R";
 case HEVC_NAL_RASL_N : return "RASL_N";
 case HEVC_NAL_RASL_R : return "RASL_R";
+case HEVC_NAL_VCL_N10: return "VCL_N10";
+case HEVC_NAL_VCL_R11: return "VCL_R11";
+case HEVC_NAL_VCL_N12: return "VCL_N12";
+case HEVC_NAL_VCL_R13: return "VLC_R13";
+case HEVC_NAL_VCL_N14: return "VCL_N14";
+case HEVC_NAL_VCL_R15: return "VCL_R15";
 case HEVC_NAL_BLA_W_LP   : return "BLA_W_LP";
 case HEVC_NAL_BLA_W_RADL : return "BLA_W_RADL";
 case HEVC_NAL_BLA_N_LP   : return "BLA_N_LP";
 case HEVC_NAL_IDR_W_RADL : return "IDR_W_RADL";
 case HEVC_NAL_IDR_N_LP   : return "IDR_N_LP";
 case HEVC_NAL_CRA_NUT: return "CRA_NUT";
+case HEVC_NAL_IRAP_VCL22 : return "IRAP_VCL22";
+case HEVC_NAL_IRAP_VCL23 : return "IRAP_VCL23";
+case HEVC_NAL_RSV_VCL24  : return "RSV_VCL24";
+case HEVC_NAL_RSV_VCL25  : return "RSV_VCL25";
+case HEVC_NAL_RSV_VCL26  : return "RSV_VCL26";
+case HEVC_NAL_RSV_VCL27  : return "RSV_VCL27";
+case HEVC_NAL_RSV_VCL28  : return "RSV_VCL28";
+case HEVC_NAL_RSV_VCL29  : return "RSV_VCL29";
+case HEVC_NAL_RSV_VCL30  : return "RSV_VCL30";
+case HEVC_NAL_RSV_VCL31  : return "RSV_VCL31";
 case HEVC_NAL_VPS: return "VPS";
 case HEVC_NAL_SPS: return "SPS";
 case HEVC_NAL_PPS: return "PPS";
-- 
2.7.4

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

[FFmpeg-devel] [PATCH 4/4] lavc/cbs_h2645: fix no slice data trigger the assert.

[Jun Zhao]

when the NALU data with zero, just give a warning.

Fixes ticket #7200

Signed-off-by: Jun Zhao 
---
 libavcodec/cbs_h2645.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index ab33cdb..08b060c 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -521,7 +521,11 @@ static int 
cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
 // Remove trailing zeroes.
 while (size > 0 && nal->data[size - 1] == 0)
 --size;
-av_assert0(size > 0);
+if (!size) {
+av_log(ctx->log_ctx, AV_LOG_WARNING, "No slice data - that was 
just the header. "
+   "Probably invalid unaligned padding on non-final NAL 
unit.\n");
+continue;
+}
 
 data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
 if (!data)
-- 
2.7.4

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

[FFmpeg-devel] [PATCH 3/4] lavc/h2645_parse: add h264_nal_unit_name for h264 NAL type.

[Jun Zhao]

Signed-off-by: Jun Zhao 
---
 libavcodec/h2645_parse.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index 579b2c9..8d67579 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -28,6 +28,7 @@
 
 #include "bytestream.h"
 #include "hevc.h"
+#include "h264.h"
 #include "h2645_parse.h"
 
 int ff_h2645_extract_rbsp(const uint8_t *src, int length,
@@ -193,6 +194,27 @@ static const char *hevc_nal_unit_name(int nal_type)
 }
 }
 
+static const char *h264_nal_unit_name(int nal_type)
+{
+switch(nal_type) {
+case H264_NAL_SLICE   : return "SLICE";
+case H264_NAL_DPA : return "DPA";
+case H264_NAL_DPB : return "DPB";
+case H264_NAL_DPC : return "DPC";
+case H264_NAL_IDR_SLICE   : return "IDR_SLICE";
+case H264_NAL_SEI : return "SEI";
+case H264_NAL_SPS : return "SPS";
+case H264_NAL_PPS : return "PPS";
+case H264_NAL_AUD : return "AUD";
+case H264_NAL_END_SEQUENCE: return "END_SEQUENCE";
+case H264_NAL_END_STREAM  : return "END_STREAM";
+case H264_NAL_FILLER_DATA : return "FILLER_DATA";
+case H264_NAL_SPS_EXT : return "SPS_EXT";
+case H264_NAL_AUXILIARY_SLICE : return "AUXILIARY_SLICE";
+default : return "?";
+}
+}
+
 static int get_bit_length(H2645NAL *nal, int skip_trailing_zeros)
 {
 int size = nal->size;
@@ -255,8 +277,8 @@ static int h264_parse_nal_header(H2645NAL *nal, void 
*logctx)
 nal->type= get_bits(gb, 5);
 
 av_log(logctx, AV_LOG_DEBUG,
-   "nal_unit_type: %d, nal_ref_idc: %d\n",
-   nal->type, nal->ref_idc);
+   "nal_unit_type: %d(%s), nal_ref_idc: %d\n",
+   nal->type, h264_nal_unit_name(nal->type), nal->ref_idc);
 
 return 1;
 }
-- 
2.7.4

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

Re: [FFmpeg-devel] [PATCH 4/4] lavc/cbs_h2645: fix no slice data trigger the assert.

[James Almer]

On 5/11/2018 2:11 AM, Jun Zhao wrote:
> when the NALU data with zero, just give a warning.
> 
> Fixes ticket #7200
> 
> Signed-off-by: Jun Zhao 
> ---
>  libavcodec/cbs_h2645.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index ab33cdb..08b060c 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -521,7 +521,11 @@ static int 
> cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>  // Remove trailing zeroes.
>  while (size > 0 && nal->data[size - 1] == 0)
>  --size;
> -av_assert0(size > 0);
> +if (!size) {
> +av_log(ctx->log_ctx, AV_LOG_WARNING, "No slice data - that was 
> just the header. "
> +   "Probably invalid unaligned padding on non-final NAL 
> unit.\n");
> +continue;
> +}

Maybe ff_h2645_packet_split() should instead just skip the NALU
altogether if after removing all trailing zero bits the size in bytes
ends up being zero?

>  
>  data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>  if (!data)
> 

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