Re: [FFmpeg-devel] [PATCH v3 07/11] avcodec: add cbs for h266/vvc

2021-01-13 Thread James Almer

On 1/11/2021 1:33 PM, Nuo Mi wrote:

@@ -1218,6 +1464,20 @@ static int cbs_h2645_unit_requires_zero_byte(enum 
AVCodecID codec_id,
  return type == H264_NAL_SPS || type == H264_NAL_PPS;
  if (codec_id == AV_CODEC_ID_HEVC)
  return type == HEVC_NAL_VPS || type == HEVC_NAL_SPS || type == 
HEVC_NAL_PPS;
+if (codec_id == AV_CODEC_ID_VVC) {
+switch (type) {
+case VVC_DCI_NUT:
+case VVC_OPI_NUT:
+case VVC_VPS_NUT:
+case VVC_SPS_NUT:
+case VVC_PPS_NUT:
+case VVC_PREFIX_APS_NUT:
+case VVC_SUFFIX_APS_NUT:
+return 1;
+default:
+return 0;
+}


return type >= VVC_OPI_NUT && type <= VVC_SUFFIX_APS_NUT;


+}
  return 0;
  }
  
@@ -1369,6 +1629,34 @@ static void cbs_h265_close(CodedBitstreamContext *ctx)

  av_buffer_unref(>pps_ref[i]);
  }
  
+static void cbs_h266_flush(CodedBitstreamContext *ctx)

+{
+CodedBitstreamH266Context *h266 = ctx->priv_data;
+
+for (int i = 0; i < FF_ARRAY_ELEMS(h266->sps); i++) {
+av_buffer_unref(>sps_ref[i]);
+h266->sps[i] = NULL;
+}
+for (int i = 0; i < FF_ARRAY_ELEMS(h266->pps); i++) {
+av_buffer_unref(>pps_ref[i]);
+h266->pps[i] = NULL;
+}
+av_buffer_unref(>ph_ref);
+h266->ph = NULL;
+
+h266->active_sps = NULL;
+h266->active_pps = NULL;
+}
+
+static void cbs_h266_close(CodedBitstreamContext *ctx)
+{
+CodedBitstreamH266Context *h266 = ctx->priv_data;
+
+cbs_h266_flush(ctx);
+ff_h2645_packet_uninit(>common.read_packet);
+
+ }
+
  static void cbs_h264_free_sei_payload(H264RawSEIPayload *payload)
  {
  switch (payload->payload_type) {
@@ -1513,6 +1801,76 @@ static const CodedBitstreamUnitTypeDescriptor 
cbs_h265_unit_types[] = {
  CBS_UNIT_TYPE_END_OF_LIST
  };
  
+static void cbs_h266_free_sei(void *opaque, uint8_t *content)

+{
+}


static void cbs_h266_free_sei_payload(H266RawSEIPayload *payload)
{
switch (payload->payload_type) {
case VVC_SEI_TYPE_DECODED_PICTURE_HASH:
break;
default:
av_buffer_unref(>payload.other.data_ref);
break;
}
av_buffer_unref(>extension_data.data_ref);
}

static void cbs_h266_free_sei(void *opaque, uint8_t *content)
{
H266RawSEI *sei = (H266RawSEI*)content;

for (int i = 0; i < sei->payload_count; i++)
cbs_h266_free_sei_payload(>payload[i]);
av_freep();
}
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avutil/tx: use ENOSYS instead of ENOTSUP

2021-01-13 Thread Lynne
Jan 14, 2021, 03:04 by jamr...@gmail.com:

> It's the standard error code used across the codebase to signal unimplemented
> or unsupported features.
>
> Signed-off-by: James Almer 
> ---
>  libavutil/tx_template.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavutil/tx_template.c b/libavutil/tx_template.c
> index a91b8f900c..155e879f8e 100644
> --- a/libavutil/tx_template.c
> +++ b/libavutil/tx_template.c
> @@ -684,7 +684,7 @@ int TX_NAME(ff_tx_init_mdct_fft)(AVTXContext *s, av_tx_fn 
> *tx,
>  * direct 3, 5 and 15 transforms as they're too niche. */
>  if (len > 1 || m == 1) {
>  if (is_mdct && (l & 1)) /* Odd (i)MDCTs are not supported yet */
> -return AVERROR(ENOTSUP);
> +return AVERROR(ENOSYS);
>

LGTM, thanks. I'll make a note to use this instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avutil/tx: use ENOSYS instead of ENOTSUP

2021-01-13 Thread James Almer
It's the standard error code used across the codebase to signal unimplemented
or unsupported features.

Signed-off-by: James Almer 
---
 libavutil/tx_template.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/tx_template.c b/libavutil/tx_template.c
index a91b8f900c..155e879f8e 100644
--- a/libavutil/tx_template.c
+++ b/libavutil/tx_template.c
@@ -684,7 +684,7 @@ int TX_NAME(ff_tx_init_mdct_fft)(AVTXContext *s, av_tx_fn 
*tx,
  * direct 3, 5 and 15 transforms as they're too niche. */
 if (len > 1 || m == 1) {
 if (is_mdct && (l & 1)) /* Odd (i)MDCTs are not supported yet */
-return AVERROR(ENOTSUP);
+return AVERROR(ENOSYS);
 s->n = l;
 s->m = 1;
 *tx = naive_fft;
-- 
2.30.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/5] ac3enc_fixed: convert to 32-bit sample format

2021-01-13 Thread Lynne
Jan 13, 2021, 00:55 by d...@lynne.ee:

> Jan 12, 2021, 22:24 by andreas.rheinha...@gmail.com:
>
>> Lynne:
>>
>>> The AC3 encoder used to be a separate library called "Aften", which
>>> got merged into libavcodec (literally, SVN commits and all).
>>> The merge preserved as much features from the library as possible.
>>>
>>> The code had two versions - a fixed point version and a floating
>>> point version. FFmpeg had floating point DSP code used by other
>>> codecs, the AC3 decoder including, so the floating-point DSP was
>>> simply replaced with FFmpeg's own functions.
>>> However, FFmpeg had no fixed-point audio code at that point. So
>>> the encoder brought along its own fixed-point DSP functions,
>>> including a fixed-point MDCT.
>>>
>>> The fixed-point MDCT itself is trivially just a float MDCT with a
>>> different type and each multiply being a fixed-point multiply.
>>> So over time, it got refactored, and the FFT used for all other codecs
>>> was templated.
>>>
>>> Due to design decisions at the time, the fixed-point version of the
>>> encoder operates at 16-bits of precision. Although convenient, this,
>>> even at the time, was inadequate and inefficient. The encoder is noisy,
>>> does not produce output comparable to the float encoder, and even
>>> rings at higher frequencies due to the badly approximated winow function.
>>>
>>> Enter MIPS (owned by Imagination Technologies at the time). They wanted
>>> quick fixed-point decoding on their FPUless cores. So they contributed
>>> patches to template the AC3 decoder so it had both a fixed-point
>>> and a floating-point version. They also did the same for the AAC decoder.
>>> They however, used 32-bit samples. Not 16-bits. And we did not have
>>> 32-bit fixed-point DSP functions, including an MDCT. But instead of
>>> templating our MDCT to output 3 versions (float, 32-bit fixed and 16-bit 
>>> fixed),
>>> they simply copy-pasted their own MDCT into ours, and completely
>>> ifdeffed our own MDCT code out if a 32-bit fixed point MDCT was selected.
>>>
>>> This is also the status quo nowadays - 2 separate MDCTs, one which
>>> produces floating point and 16-bit fixed point versions, and one
>>> sort-of integrated which produces 32-bit MDCT.
>>>
>>> MIPS weren't all that interested in encoding, so they left the encoder
>>> as-is, and they didn't care much about the ifdeffery, mess or quality - it's
>>> not their problem.
>>>
>>> So the MDCT/FFT code has always been a thorn in anyone looking to clean up
>>> code's eye.
>>>
>>> Backstory over. Internally AC3 operates on 25-bit fixed-point coefficients.
>>> So for the floating point version, the encoder simply runs the float MDCT,
>>> and converts the resulting coefficients to 25-bit fixed-point, as AC3 is 
>>> inherently
>>> a fixed-point codec. For the fixed-point version, the input is 16-bit 
>>> samples,
>>> so to maximize precision the frame samples are analyzed and the highest set
>>> bit is detected via ac3_max_msb_abs_int16(), and the coefficients are then
>>> scaled up via ac3_lshift_int16(), so the input for the FFT is always at 
>>> least 14 bits,
>>> computed in normalize_samples(). After FFT, the coefficients are scaled up 
>>> to 25 bits.
>>>
>>> This patch simply changes the encoder to accept 32-bit samples, reusing
>>> the already well-optimized 32-bit MDCT code, allowing us to clean up and 
>>> drop
>>> a large part of a very messy code of ours, as well as prepare for the 
>>> future lavu/tx
>>> conversion. The coefficients are simply scaled down to 25 bits during 
>>> windowing,
>>> skipping 2 separate scalings, as the hacks to extend precision are simply 
>>> no longer
>>> necessary. There's no point in running the MDCT always at 32 bits when 
>>> you're
>>> going to drop 6 bits off anyway, the headroom is plenty, and the MDCT rounds
>>> properly.
>>>
>>> This also makes the encoder even slightly more accurate over the float 
>>> version,
>>> as there's no coefficient conversion step necessary.
>>>
>>> SIZE SAVINGS:
>>> ARM32:
>>> HARDCODED TABLES:
>>> BASE   - 10709590
>>> DROP  DSP  - 10702872 - diff:   -6.56KiB
>>> DROP  MDCT - 10667932 - diff:  -34.12KiB - both:   -40.68KiB
>>> DROP  FFT  - 10336652 - diff: -323.52KiB - all:   -364.20KiB
>>> SOFTCODED TABLES:
>>> BASE   -  9685096
>>> DROP  DSP  -  9678378 - diff:   -6.56KiB
>>> DROP  MDCT -  9643466 - diff:  -34.09KiB - both:   -40.65KiB
>>> DROP  FFT  -  9573918 - diff:  -67.92KiB - all:   -108.57KiB
>>>
>>> ARM64:
>>> HARDCODED TABLES:
>>> BASE   - 14641112
>>> DROP  DSP  - 14633806 - diff:   -7.13KiB
>>> DROP  MDCT - 14604812 - diff:  -28.31KiB - both:   -35.45KiB
>>> DROP  FFT  - 14286826 - diff: -310.53KiB - all:   -345.98KiB
>>> SOFTCODED TABLES:
>>> BASE   - 13636238
>>> DROP  DSP  - 13628932 - diff:   -7.13KiB
>>> DROP  MDCT - 13599866 - diff:  -28.38KiB - both:   -35.52KiB
>>> DROP  FFT  - 13542080 - diff:  -56.43KiB - all:    -91.95KiB
>>>
>>> x86:

Re: [FFmpeg-devel] [PATCH] avformat/fifo: utilize a clone packet for muxing

2021-01-13 Thread Jan Ekström
On Tue, Jan 12, 2021 at 9:59 AM Jan Ekström  wrote:
>
> On Mon, Jan 11, 2021 at 10:53 PM Andreas Rheinhardt
>  wrote:
> >
> > Jan Ekström:
> > > On Tue, Dec 8, 2020 at 2:54 PM Jan Ekström  wrote:
> > >>
> > >> From: Jan Ekström 
> > >>
> > >> This way the timestamp adjustments do not have to be manually
> > >> undone in case of failure and need to recover/retry.
> > >>
> > >> Fixes an issue where the timestamp adjustment would be re-done over
> > >> and over again when recovery by muxing the same AVPacket again is
> > >> attempted. Would become visible if the fifo muxer's time base and
> > >> the output muxer's time base do not match (by the value either
> > >> becoming smaller and smaller, or larger and larger).
> > >>
> > >> Signed-off-by: Jan Ekström 
> > >
> > > Ping.
> > >
> > > Unless someone finds some disgruntling points in this patch, I will
> > > soon move towards applying this.
> > >
> > > My initial plan was to make a simplified v2 where the output AVPacket
> > > was on stack limited to the fifo_thread_write_packet context, but
> > > apparently gradual removal of stack usage of AVPackets is being
> > > planned, so I decided to not to.
> > >
> > > Best regards,
> > > Jan
> >
> > Can't you just record (in FifoMessage) whether the timestamps have
> > already been converted to the desired timebase?
>
> Back when I found this issue in this function that aligns the time
> bases and writes the packet on the underlying avformat context, I did
> not think of that, but I did think of reversing the time base scaling
> in case of failure. That I then opted not to do due to possibly being
> inaccurate unless I saved all of those values from the AVPacket that
> av_packet_rescale_ts touches. Thus, I settled on just utilizing a
> reference/copy for the actual muxing, so that it could be easily
> discarded and the original AVPacket's values were not lost.
>
> > (Or why not just copy
> > the timebase chosen by the internal muxer to the user-visible stream so
> > that we don't even have to convert it? This is how muxers always operate.)
>
> This one sounds more like the correct way in the end, if possible. My
> attempt for now was to just fix an issue within the current logic
> (time base handling in case of failure was forgotten). I am trying to
> remind myself at which point the AVStreams should no longer be
> poked/modified as far as time base is concerned... init or header?
> init seems to call init_pts in libavformat/mux.c, so my initial
> guesstimate is that where fifo.c is currently doing its things
> (fifo_mux_init), you could just add the time base sync. Most API
> clients tend to call only avformat_write_header so in that sense with
> various API clients you probably wouldn't even notice the difference
> :) .

So I just double-checked the docs in avformat.h, and the point of
update given to the API user is the header writing, which in the fifo
muxer is being done asynchronously in its own thread. We would have to
basically make fifo_write_header wait until the first time
avformat_write_header gets successfully called in
fifo_thread_write_header.

While possible - and could make the time base juggling unnecessary -
not sure if it would be my first attempt. :) I'd probably first want
to get the time base logic fixed since that would be limited to
fifo_thread_write_packet.

If there were question marks on why I utilized a separate reference
AVPacket, it was mostly to keep the time related values original in
the fed AVPacket, thus backing up all of the time values that
av_packet_rescale_ts might touch (if new time fields were to be added
to AVPacket, and modified by av_packet_rescale_ts, then the code would
have to be updated if I had just backed up the values manually and
then passed them back). In addition to possibly causing a new buffer
to be allocated in case of a non-refcounted buffer (although the fifo
muxer does do an av_packet_ref for all input packets, so everything
fed in through the worker thread should be reference counted), It does
cause the side data etc to be copied as well, which is of course
unfortunate. In the testing that was done this did not seem to cause
major issues, though.

If someone feels heavily about this, I can of course write
ff_packet_copy_ts(AVPacket *dst, const AVPacket *src), which could
then be utilized against the dummy AVPacket.

Jan


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.

2021-01-13 Thread Michael Niedermayer
On Mon, Jan 11, 2021 at 05:46:31PM +0100, Alan Kelly wrote:
> ---
>  Fixes a bug where if there is no offset and a tail which is not processed by 
> the
>  sse3/avx2 version the dither is modified
>  Deletes mmx/mmxext yuv2yuvX version from swscale_template and adds it
>  to yuv2yuvX.asm to reduce code duplication and so that it may be used
>  to process the tail from the larger cardinal simd versions.
>  src argument of yuv2yuvX_* is now srcOffset, so that tails and offsets
>  are accounted for correctly.
>  Changes input size in checkasm so that this corner case is tested.
> 
>  libswscale/x86/Makefile   |   1 +
>  libswscale/x86/swscale.c  | 130 
>  libswscale/x86/swscale_template.c |  82 --
>  libswscale/x86/yuv2yuvX.asm   | 136 ++
>  tests/checkasm/sw_scale.c | 100 ++
>  5 files changed, 291 insertions(+), 158 deletions(-)
>  create mode 100644 libswscale/x86/yuv2yuvX.asm

This seems to be crashing again unless i messed up testing 

(gdb) disassemble $rip-32,$rip+32
Dump of assembler code from 0x55572f02 to 0x55572f42:
   0x55572f02 :   int$0x71
   0x55572f04 :   out%al,$0x3
   0x55572f06 :   vpsraw $0x3,%ymm1,%ymm1
   0x55572f0b :   vpackuswb %ymm4,%ymm3,%ymm3
   0x55572f0f :   vpackuswb %ymm1,%ymm6,%ymm6
   0x55572f13 :   mov(%rdi),%rdx
   0x55572f16 :   vpermq $0xd8,%ymm3,%ymm3
   0x55572f1c :   vpermq $0xd8,%ymm6,%ymm6
=> 0x55572f22 :   vmovdqa %ymm3,(%rcx,%rax,1)
   0x55572f27 :   vmovdqa %ymm6,0x20(%rcx,%rax,1)
   0x55572f2d :   add$0x40,%rax
   0x55572f31 :   mov%rdi,%rsi
   0x55572f34 :   cmp%r8,%rax
   0x55572f37 :   jb 0x55572eae 

   0x55572f3d :   vzeroupper 
   0x55572f40 :   retq   
   0x55572f41 :   nopw   %cs:0x0(%rax,%rax,1)
   
rax0x0  0
rbx0x30 48
rcx0x5583f470   93824995292272
rdx0x5585e500   93824995419392

#0  0x55572f22 in ff_yuv2yuvX_avx2 ()
#1  0x555724ee in yuv2yuvX_avx2 ()
#2  0x5556b4f6 in chr_planar_vscale ()
#3  0x55566d41 in swscale ()
#4  0x55568284 in sws_scale ()



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

What does censorship reveal? It reveals fear. -- Julian Assange


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/3] kmsgrab: Use invalid modifier if modifiers weren't used.

2021-01-13 Thread Mark Thompson

On 13/01/2021 00:19, Bas Nieuwenhuizen wrote:

A friendly ping on reviewing this series. Thanks!


Apologies; last time I looked at this I got distracted by Intel having broken import 
of X-tiled surfaces from kmsgrab, which was a bit of a problem to my testing.  
(, since fixed.)

Patches 1 and 2 look good, applied.

Thanks,

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 3/3] hwcontext_vaapi: Use PRIME_2 memory type for modifiers.

2021-01-13 Thread Mark Thompson

On 13/11/2020 23:15, Bas Nieuwenhuizen wrote:

This way we can pass explicit modifiers in. Sometimes the
modifier matters for the number of memory planes that
libva accepts, in particular when dealing with
driver-compressed textures. Furthermore the driver might
not actually be able to determine the implicit modifier
if all the buffer-passing has used explicit modifier.
All these issues should be resolved by passing in the
modifier, and for that we switch to using the PRIME_2
memory type.

Tested with experimental radeonsi patches for modifiers
and kmsgrab. Also tested with radeonsi without the
patches to double-check it works without PRIME_2 support.

v2:
   Cache PRIME_2 support to avoid doing two calls every time on
   libva drivers that do not support it.
---
  libavutil/hwcontext_vaapi.c | 158 ++--
  1 file changed, 115 insertions(+), 43 deletions(-)

diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 2227d6ed69..62b5a163ee 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -79,6 +79,9 @@ typedef struct VAAPIFramesContext {
  unsigned int rt_format;
  // Whether vaDeriveImage works.
  int derive_works;
+// Caches whether VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME_2 is unsupported for
+// surface imports.
+int prime_2_import_unsupported;
  } VAAPIFramesContext;
  
  typedef struct VAAPIMapping {

@@ -1022,6 +1025,7 @@ static void vaapi_unmap_from_drm(AVHWFramesContext 
*dst_fc,
  static int vaapi_map_from_drm(AVHWFramesContext *src_fc, AVFrame *dst,
const AVFrame *src, int flags)
  {
+VAAPIFramesContext *src_vafc = src_fc->internal->priv;
  AVHWFramesContext  *dst_fc =
  (AVHWFramesContext*)dst->hw_frames_ctx->data;
  AVVAAPIDeviceContext  *dst_dev = dst_fc->device_ctx->hwctx;
@@ -1029,25 +1033,10 @@ static int vaapi_map_from_drm(AVHWFramesContext 
*src_fc, AVFrame *dst,
  const VAAPIFormatDescriptor *format_desc;
  VASurfaceID surface_id;
  VAStatus vas;
+VAStatus prime2_vas = VA_STATUS_SUCCESS;
+int use_prime2;
  uint32_t va_fourcc;
-int err, i, j, k;
-
-unsigned long buffer_handle;
-VASurfaceAttribExternalBuffers buffer_desc;
-VASurfaceAttrib attrs[2] = {
-{
-.type  = VASurfaceAttribMemoryType,
-.flags = VA_SURFACE_ATTRIB_SETTABLE,
-.value.type= VAGenericValueTypeInteger,
-.value.value.i = VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME,
-},
-{
-.type  = VASurfaceAttribExternalBufferDescriptor,
-.flags = VA_SURFACE_ATTRIB_SETTABLE,
-.value.type= VAGenericValueTypePointer,
-.value.value.p = _desc,
-}
-};
+int err, i, j;
  
  desc = (AVDRMFrameDescriptor*)src->data[0];
  
@@ -1083,35 +1072,115 @@ static int vaapi_map_from_drm(AVHWFramesContext *src_fc, AVFrame *dst,

  format_desc = vaapi_format_from_fourcc(va_fourcc);
  av_assert0(format_desc);
  
-buffer_handle = desc->objects[0].fd;

-buffer_desc.pixel_format = va_fourcc;
-buffer_desc.width= src_fc->width;
-buffer_desc.height   = src_fc->height;
-buffer_desc.data_size= desc->objects[0].size;
-buffer_desc.buffers  = _handle;
-buffer_desc.num_buffers  = 1;
-buffer_desc.flags= 0;
-
-k = 0;
-for (i = 0; i < desc->nb_layers; i++) {
-for (j = 0; j < desc->layers[i].nb_planes; j++) {
-buffer_desc.pitches[k] = desc->layers[i].planes[j].pitch;
-buffer_desc.offsets[k] = desc->layers[i].planes[j].offset;
-++k;
+use_prime2 = !src_vafc->prime_2_import_unsupported &&
+ desc->objects[0].format_modifier != DRM_FORMAT_MOD_INVALID;
+if (use_prime2) {
+VADRMPRIMESurfaceDescriptor prime_desc;
+VASurfaceAttrib prime_attrs[2] = {
+{
+.type  = VASurfaceAttribMemoryType,
+.flags = VA_SURFACE_ATTRIB_SETTABLE,
+.value.type= VAGenericValueTypeInteger,
+.value.value.i = VA_SURFACE_ATTRIB_MEM_TYPE_DRM_PRIME_2,
+},
+{
+.type  = VASurfaceAttribExternalBufferDescriptor,
+.flags = VA_SURFACE_ATTRIB_SETTABLE,
+.value.type= VAGenericValueTypePointer,
+.value.value.p = _desc,
+}
+};
+prime_desc.fourcc = va_fourcc;
+prime_desc.width = src_fc->width;
+prime_desc.height = src_fc->height;
+prime_desc.num_objects = desc->nb_objects;
+for (i = 0; i < desc->nb_objects; ++i) {
+prime_desc.objects[i].fd = desc->objects[i].fd;
+prime_desc.objects[i].size = desc->objects[i].size;
+prime_desc.objects[i].drm_format_modifier =
+desc->objects[i].format_modifier;
+}
+
+prime_desc.num_layers = desc->nb_layers;

Re: [FFmpeg-devel] [PATCH v3 07/11] avcodec: add cbs for h266/vvc

2021-01-13 Thread James Almer

On 1/12/2021 11:34 PM, James Almer wrote:

+    if (err < 0)
+    return err;
+    }
+    break;
+
+    case VVC_TRAIL_NUT:
+    case VVC_STSA_NUT:
+    case VVC_RADL_NUT:
+    case VVC_RASL_NUT:
+    case VVC_IDR_W_RADL:
+    case VVC_IDR_N_LP:
+    case VVC_CRA_NUT:
+    case VVC_GDR_NUT:
+    {
+    H266RawSlice *slice = unit->content;
+    int pos, len;
+
+    err = cbs_h266_read_slice_header(ctx, , >header);
+    if (err < 0)
+    return err;


Add a call to cbs_h266_replace_ph() here when 
slice->header.sh_picture_header_in_slice_header_flag is true, and pass a 
pointer to slice->header.sh_picture_header to it.


Actually no, this wouldn't work as is in the cases where 
ff_cbs_make_unit_refcounted() makes it refcounted. Neither slice or the 
pointer passed to  cbs_h266_replace_ph() would be valid.


You'd have ensure the slice variable points the new unit->content after 
a successful call to cbs_h266_replace_ph() (Which you can leave 
untouched), and then make priv->ph point to the H266RawPH struct within 
slice.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread James Almer

On 1/13/2021 4:58 PM, Thierry Foucu wrote:

HI Lynne

On Wed, Jan 13, 2021 at 10:30 AM Lynne  wrote:


Jan 13, 2021, 18:46 by tfo...@gmail.com:


By default the time code scale in a MKV file in millisecond. With this
option we can set the time code scale to microsecond or nanoseconds for
very high frame rate.
---
  libavformat/matroskaenc.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 233c472b8f..cfad6a4693 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
  int default_mode;

  uint32_tsegment_uid[4];
+
+int64_t timecodescale;
  } MatroskaMuxContext;

  /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
@@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
  return ret;
  pb = mkv->info.bc;

-put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
+put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
  if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
  put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
  if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
@@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
  // after 4k and on a keyframe
  if (IS_SEEKABLE(pb, mkv)) {
  if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 5000;
+mkv->cluster_time_limit = 5*(10/mkv->timecodescale);
  if (mkv->cluster_size_limit < 0)
  mkv->cluster_size_limit = 5 * 1024 * 1024;
  } else {
  if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 1000;
+mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
  if (mkv->cluster_size_limit < 0)
  mkv->cluster_size_limit = 32 * 1024;
  }
@@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
  }

  // ms precision is the de-facto standard timescale for mkv files
-avpriv_set_pts_info(st, 64, 1, 1000);
+avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);

  if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
  if (mkv->mode == MODE_WEBM) {
@@ -2795,6 +2797,7 @@ static const AVOption options[] = {
  { "infer", "For each track type, mark the first track of disposition

default as default; if none exists, mark the first track as default.", 0,
AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS,
"default_mode" },

  { "infer_no_subs", "For each track type, mark the first track of

disposition default as default; for audio and video: if none exists, mark
the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },

  { "passthrough", "Use the disposition flag as-is", 0,

AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
"default_mode" },

+{ "timecodescale", "Time code scale for all tracks in nanoseconds",

OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1,
10, FLAGS },

  { NULL },
  };



This, x1!
Can we make it 1ns by default? Or maybe autodetect the highest precision
timebase of all streams and use it?



The MKV spec seems to indicate a default value of ms. I did not want to
change it in this patch.



Let's not keep generating garbage files with shitty rounding that players
have to work around to recover the original timestamp based on framerate
to reduce jitter (mpv does this!).



+1 on this, but I'm worry, we could break a lot of decoder which may not
follow the correct timecodescale (Which we found out in our system a while
back)



Also WebM mode should disable this, because the spec is dumb and specifies
a 1ms precision for no good reason.



If we keep the default value of ms, then this will not change any webm
unless a user decides to generate a Webm file and set this flag.
Do you want me to log an error saying that using this flag to set something
other than ms for webm is incorrect?


The webm spec states "The TimecodeScale element SHOULD be set to a 
default of 1.000.000 nanoseconds.", so it's not forbidden per se.
Does libwebm handle files with a different TimecodeScale than the 
default? If so, then it's fine. Otherwise, I agree we should probably 
error out.






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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".






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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 2/3] avformat/udp: add memory alloc checks

2021-01-13 Thread Marton Balint



On Sat, 9 Jan 2021, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
libavformat/udp.c | 4 
1 file changed, 4 insertions(+)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 088e30c..a7fbb94 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -903,6 +903,10 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)
if ((!is_output && s->circular_buffer_size) || (is_output && s->bitrate && 
s->circular_buffer_size)) {
/* start the task going */
s->fifo = av_fifo_alloc(s->circular_buffer_size);
+if (!s->fifo) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
ret = pthread_mutex_init(>mutex, NULL);
if (ret != 0) {
av_log(h, AV_LOG_ERROR, "pthread_mutex_init failed : %s\n", 
strerror(ret));


LGTM, thanks.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v7] avformat/udp: return the error code instead of generic EIO

2021-01-13 Thread Marton Balint



On Wed, 13 Jan 2021, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
libavformat/udp.c | 58 ++-
1 file changed, 36 insertions(+), 22 deletions(-)


LGTM, thanks.

Marton



diff --git a/libavformat/udp.c b/libavformat/udp.c
index 13c346a..c4e403b 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -165,7 +165,7 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
if (addr->sa_family == AF_INET) {
if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, , 
sizeof(mcastTTL)) < 0) {
ff_log_net_error(NULL, AV_LOG_ERROR, 
"setsockopt(IP_MULTICAST_TTL)");
-return -1;
+return ff_neterrno();
}
}
#endif
@@ -173,7 +173,7 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
if (addr->sa_family == AF_INET6) {
if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, , 
sizeof(mcastTTL)) < 0) {
ff_log_net_error(NULL, AV_LOG_ERROR, 
"setsockopt(IPV6_MULTICAST_HOPS)");
-return -1;
+return ff_neterrno();
}
}
#endif
@@ -193,7 +193,7 @@ static int udp_join_multicast_group(int sockfd, struct 
sockaddr *addr,struct soc
mreq.imr_interface.s_addr = INADDR_ANY;
if (setsockopt(sockfd, IPPROTO_IP, IP_ADD_MEMBERSHIP, (const void *), 
sizeof(mreq)) < 0) {
ff_log_net_error(NULL, AV_LOG_ERROR, 
"setsockopt(IP_ADD_MEMBERSHIP)");
-return -1;
+return ff_neterrno();
}
}
#endif
@@ -206,7 +206,7 @@ static int udp_join_multicast_group(int sockfd, struct 
sockaddr *addr,struct soc
mreq6.ipv6mr_interface = 0;
if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, , 
sizeof(mreq6)) < 0) {
ff_log_net_error(NULL, AV_LOG_ERROR, 
"setsockopt(IPV6_ADD_MEMBERSHIP)");
-return -1;
+return ff_neterrno();
}
}
#endif
@@ -633,6 +633,7 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)
char buf[256];
struct sockaddr_storage my_addr;
socklen_t len;
+int ret;

h->is_streamed = 1;

@@ -641,12 +642,12 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)
s->buffer_size = is_output ? UDP_TX_BUF_SIZE : UDP_RX_BUF_SIZE;

if (s->sources) {
-if (ff_ip_parse_sources(h, s->sources, >filters) < 0)
+if ((ret = ff_ip_parse_sources(h, s->sources, >filters)) < 0)
goto fail;
}

if (s->block) {
-if (ff_ip_parse_blocks(h, s->block, >filters) < 0)
+if ((ret = ff_ip_parse_blocks(h, s->block, >filters)) < 0)
goto fail;
}

@@ -712,11 +713,11 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)
av_strlcpy(localaddr, buf, sizeof(localaddr));
}
if (av_find_info_tag(buf, sizeof(buf), "sources", p)) {
-if (ff_ip_parse_sources(h, buf, >filters) < 0)
+if ((ret = ff_ip_parse_sources(h, buf, >filters)) < 0)
goto fail;
}
if (av_find_info_tag(buf, sizeof(buf), "block", p)) {
-if (ff_ip_parse_blocks(h, buf, >filters) < 0)
+if ((ret = ff_ip_parse_blocks(h, buf, >filters)) < 0)
goto fail;
}
if (!is_output && av_find_info_tag(buf, sizeof(buf), "timeout", p))
@@ -742,7 +743,7 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)
if (!(flags & AVIO_FLAG_READ))
goto fail;
} else {
-if (ff_udp_set_remote_url(h, uri) < 0)
+if ((ret = ff_udp_set_remote_url(h, uri)) < 0)
goto fail;
}

@@ -763,15 +764,22 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)
 */
if (s->reuse_socket > 0 || (s->is_multicast && s->reuse_socket < 0)) {
s->reuse_socket = 1;
-if (setsockopt (udp_fd, SOL_SOCKET, SO_REUSEADDR, &(s->reuse_socket), 
sizeof(s->reuse_socket)) != 0)
+if (setsockopt (udp_fd, SOL_SOCKET, SO_REUSEADDR, &(s->reuse_socket), 
sizeof(s->reuse_socket)) != 0) {
+ret = ff_neterrno();
goto fail;
+}
}

if (s->is_broadcast) {
#ifdef SO_BROADCAST
-if (setsockopt (udp_fd, SOL_SOCKET, SO_BROADCAST, &(s->is_broadcast), 
sizeof(s->is_broadcast)) != 0)
+if (setsockopt (udp_fd, SOL_SOCKET, SO_BROADCAST, &(s->is_broadcast), 
sizeof(s->is_broadcast)) != 0) {
+ret = ff_neterrno();
+goto fail;
+}
+#else
+ret = AVERROR(ENOSYS);
+goto fail;
#endif
-   goto fail;
}

/* Set the checksum coverage for UDP-Lite (RFC 3828) for sending and 
receiving.
@@ -788,8 +796,10 @@ static int udp_open(URLContext *h, const char *uri, int 
flags)

if (dscp >= 0) {
dscp <<= 2;
-if (setsockopt (udp_fd, IPPROTO_IP, IP_TOS, , sizeof(dscp)) != 0)
+if (setsockopt (udp_fd, IPPROTO_IP, IP_TOS, , sizeof(dscp)) != 0) 
{
+ret = ff_neterrno();

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread Lynne
Jan 13, 2021, 20:58 by tfo...@gmail.com:

> HI Lynne
>
> On Wed, Jan 13, 2021 at 10:30 AM Lynne <> d...@lynne.ee> > wrote:
>
>> Jan 13, 2021, 18:46 by >> tfo...@gmail.com>> :
>>
>> > By default the time code scale in a MKV file in millisecond. With this
>> > option we can set the time code scale to microsecond or nanoseconds for
>> > very high frame rate.
>> > ---
>> >  libavformat/matroskaenc.c | 11 +++
>> >  1 file changed, 7 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> > index 233c472b8f..cfad6a4693 100644
>> > --- a/libavformat/matroskaenc.c
>> > +++ b/libavformat/matroskaenc.c
>> > @@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
>> >  int default_mode;
>> >
>> >  uint32_tsegment_uid[4];
>> > +
>> > +int64_t timecodescale;
>> >  } MatroskaMuxContext;
>> >
>> >  /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
>> > @@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
>> >  return ret;
>> >  pb = mkv->info.bc;
>> >
>> > -put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
>> > +put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
>> >  if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
>> >  put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
>> >  if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
>> > @@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
>> >  // after 4k and on a keyframe
>> >  if (IS_SEEKABLE(pb, mkv)) {
>> >  if (mkv->cluster_time_limit < 0)
>> > -mkv->cluster_time_limit = 5000;
>> > +mkv->cluster_time_limit = 5*(10/mkv->timecodescale);
>> >  if (mkv->cluster_size_limit < 0)
>> >  mkv->cluster_size_limit = 5 * 1024 * 1024;
>> >  } else {
>> >  if (mkv->cluster_time_limit < 0)
>> > -mkv->cluster_time_limit = 1000;
>> > +mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
>> >  if (mkv->cluster_size_limit < 0)
>> >  mkv->cluster_size_limit = 32 * 1024;
>> >  }
>> > @@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
>> >  }
>> >
>> >  // ms precision is the de-facto standard timescale for mkv files
>> > -avpriv_set_pts_info(st, 64, 1, 1000);
>> > +avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);
>> >
>> >  if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
>> >  if (mkv->mode == MODE_WEBM) {
>> > @@ -2795,6 +2797,7 @@ static const AVOption options[] = {
>> >  { "infer", "For each track type, mark the first track of disposition
>> default as default; if none exists, mark the first track as default.", 0,
>> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS,
>> "default_mode" },
>> >  { "infer_no_subs", "For each track type, mark the first track of
>> disposition default as default; for audio and video: if none exists, mark
>> the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
>> DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
>> >  { "passthrough", "Use the disposition flag as-is", 0,
>> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
>> "default_mode" },
>> > +{ "timecodescale", "Time code scale for all tracks in nanoseconds",
>> OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1,
>> 10, FLAGS },
>> >  { NULL },
>> >  };
>> >
>>
>> This, x1!
>> Can we make it 1ns by default? Or maybe autodetect the highest precision
>> timebase of all streams and use it?
>>
>
> The MKV spec seems to indicate a default value of ms. I did not want to
> change it in this patch.
>

The spec was written in 2001ish. Mobile phones didn't even exist back then,
I think, and internet video must have been 12fps mpeg2. 1ms didn't make
much sense back then, and it makes exactly no sense at at all right now
with phones regularly shooting at hundreds of frames per second.


>> Let's not keep generating garbage files with shitty rounding that players
>> have to work around to recover the original timestamp based on framerate
>> to reduce jitter (mpv does this!).
>>
>
> +1 on this, but I'm worry, we could break a lot of decoder which may not
> follow the correct timecodescale (Which we found out in our system a while
> back)
>

I think that's a bug in their code, and we should help them fix it by changing
the default.
It's kind of important, as nothing will change unless we start to generate files
with better precision by default. For decades the main complaint about
Matroska has been the ridiculously low timestamp precision most files have. Even
the spec authors agree. There were talks to add hacks to correct the precision
of the timestamps in a compatible way with buggy demuxers some years ago, but
nothing came of it.

I'd really like to not have to tell users "please enable this magical demuxer 
option
to not mess up high framerate files or induce jitter". I'd be so much better to 
say
"oh, your files don't 

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread James Almer

On 1/13/2021 5:50 PM, Thierry Foucu wrote:

a note with this change:
If we set the timecodescale to microsecond, and we encode a 30 fps video,
the duration of each frame are then 3 us.
In this case,
(int16_t)cluster_time != cluster_time
Will almost every time faile and we will need to create a new block per
frame (it seems to me at least)
Because in the block header, there is a timestamp relative to Cluster
timestamp (signed int16) which cannot represent 3

On Wed, Jan 13, 2021 at 12:02 PM James Almer  wrote:


On 1/13/2021 2:46 PM, Thierry Foucu wrote:

By default the time code scale in a MKV file in millisecond. With this
option we can set the time code scale to microsecond or nanoseconds for
very high frame rate.
---
   libavformat/matroskaenc.c | 11 +++
   1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 233c472b8f..cfad6a4693 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
   int default_mode;

   uint32_tsegment_uid[4];
+
+int64_t timecodescale;
   } MatroskaMuxContext;

   /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
@@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
   return ret;
   pb = mkv->info.bc;

-put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
+put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
   if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
   put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
   if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
@@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
   // after 4k and on a keyframe
   if (IS_SEEKABLE(pb, mkv)) {
   if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 5000;
+mkv->cluster_time_limit = 5*(10/mkv->timecodescale);


Either 10LL, or use av_rescale(). Same for the two below


   if (mkv->cluster_size_limit < 0)
   mkv->cluster_size_limit = 5 * 1024 * 1024;
   } else {
   if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 1000;
+mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
   if (mkv->cluster_size_limit < 0)
   mkv->cluster_size_limit = 32 * 1024;
   }
@@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
   }

   // ms precision is the de-facto standard timescale for mkv

files

-avpriv_set_pts_info(st, 64, 1, 1000);
+avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);

   if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
   if (mkv->mode == MODE_WEBM) {
@@ -2795,6 +2797,7 @@ static const AVOption options[] = {
   { "infer", "For each track type, mark the first track of

disposition default as default; if none exists, mark the first track as
default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0,
FLAGS, "default_mode" },

   { "infer_no_subs", "For each track type, mark the first track of

disposition default as default; for audio and video: if none exists, mark
the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },

   { "passthrough", "Use the disposition flag as-is", 0,

AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
"default_mode" },

+{ "timecodescale", "Time code scale for all tracks in nanoseconds",

OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1,
10, FLAGS },

   { NULL },
   };


Does this cover all cases? A timecodescale of 100 was implied until
now, so many parts of the code could be hardcoding it in a non obvious way.

I see for example MATROSKA_ID_DURATION is set as av_rescale(s->duration,
1000, AV_TIME_BASE). Should that AV_TIME_BASE be changed to
mkv->timecodescale? The spec says "Duration of the Segment in
nanoseconds based on TimestampScale."



Interesting. I did not have to change this because if I check the Metadata
DURATION coming from the demuxer with a default ms and set to microsecond,
I do see in both:
DURATION: 00:00:01.00100

or is it another duration I need to look at, like this one:
https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/matroskaenc.c#L1863


That and the one immediately after it are the ones i was talking about, 
yes. In any case, it seems to be a temporary value written to the output 
that will afterwards be overwritten in 
https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/matroskaenc.c#L2559 
when writing the trailer.


The commit that added the code in mkv_write_header() is 70c1647a350. I 
guess you could ensure the temp value written there is correct by 
forcing the muxing process to stop without calling mkv_write_trailer(), 

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread Thierry Foucu
a note with this change:
If we set the timecodescale to microsecond, and we encode a 30 fps video,
the duration of each frame are then 3 us.
In this case,
(int16_t)cluster_time != cluster_time
Will almost every time faile and we will need to create a new block per
frame (it seems to me at least)
Because in the block header, there is a timestamp relative to Cluster
timestamp (signed int16) which cannot represent 3

On Wed, Jan 13, 2021 at 12:02 PM James Almer  wrote:

> On 1/13/2021 2:46 PM, Thierry Foucu wrote:
> > By default the time code scale in a MKV file in millisecond. With this
> > option we can set the time code scale to microsecond or nanoseconds for
> > very high frame rate.
> > ---
> >   libavformat/matroskaenc.c | 11 +++
> >   1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 233c472b8f..cfad6a4693 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
> >   int default_mode;
> >
> >   uint32_tsegment_uid[4];
> > +
> > +int64_t timecodescale;
> >   } MatroskaMuxContext;
> >
> >   /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
> > @@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
> >   return ret;
> >   pb = mkv->info.bc;
> >
> > -put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
> > +put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
> >   if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
> >   put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
> >   if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
> > @@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
> >   // after 4k and on a keyframe
> >   if (IS_SEEKABLE(pb, mkv)) {
> >   if (mkv->cluster_time_limit < 0)
> > -mkv->cluster_time_limit = 5000;
> > +mkv->cluster_time_limit = 5*(10/mkv->timecodescale);
>
> Either 10LL, or use av_rescale(). Same for the two below
>
> >   if (mkv->cluster_size_limit < 0)
> >   mkv->cluster_size_limit = 5 * 1024 * 1024;
> >   } else {
> >   if (mkv->cluster_time_limit < 0)
> > -mkv->cluster_time_limit = 1000;
> > +mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
> >   if (mkv->cluster_size_limit < 0)
> >   mkv->cluster_size_limit = 32 * 1024;
> >   }
> > @@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
> >   }
> >
> >   // ms precision is the de-facto standard timescale for mkv
> files
> > -avpriv_set_pts_info(st, 64, 1, 1000);
> > +avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);
> >
> >   if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
> >   if (mkv->mode == MODE_WEBM) {
> > @@ -2795,6 +2797,7 @@ static const AVOption options[] = {
> >   { "infer", "For each track type, mark the first track of
> disposition default as default; if none exists, mark the first track as
> default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0,
> FLAGS, "default_mode" },
> >   { "infer_no_subs", "For each track type, mark the first track of
> disposition default as default; for audio and video: if none exists, mark
> the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
> DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
> >   { "passthrough", "Use the disposition flag as-is", 0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
> "default_mode" },
> > +{ "timecodescale", "Time code scale for all tracks in nanoseconds",
> OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1,
> 10, FLAGS },
> >   { NULL },
> >   };
>
> Does this cover all cases? A timecodescale of 100 was implied until
> now, so many parts of the code could be hardcoding it in a non obvious way.
>
> I see for example MATROSKA_ID_DURATION is set as av_rescale(s->duration,
> 1000, AV_TIME_BASE). Should that AV_TIME_BASE be changed to
> mkv->timecodescale? The spec says "Duration of the Segment in
> nanoseconds based on TimestampScale."
>

Interesting. I did not have to change this because if I check the Metadata
DURATION coming from the demuxer with a default ms and set to microsecond,
I do see in both:
DURATION: 00:00:01.00100

or is it another duration I need to look at, like this one:
https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/matroskaenc.c#L1863

See output:
# default behavoir
./ffmpeg -f lavfi -i "testsrc=r=3/1001" -debug_ts -t 1 -y -f matroska
/tmp/test.mkv
./ffmpeg -i /tmp/test.mkv
ffmpeg version N-100413-g3799e77f93 Copyright (c) 2000-2020 the FFmpeg
developers
  built with gcc 10 (Debian 10.2.0-19)
  configuration: --disable-optimizations
  libavutil  

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread Thierry Foucu
HI Lynne

On Wed, Jan 13, 2021 at 10:30 AM Lynne  wrote:

> Jan 13, 2021, 18:46 by tfo...@gmail.com:
>
> > By default the time code scale in a MKV file in millisecond. With this
> > option we can set the time code scale to microsecond or nanoseconds for
> > very high frame rate.
> > ---
> >  libavformat/matroskaenc.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 233c472b8f..cfad6a4693 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
> >  int default_mode;
> >
> >  uint32_tsegment_uid[4];
> > +
> > +int64_t timecodescale;
> >  } MatroskaMuxContext;
> >
> >  /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
> > @@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
> >  return ret;
> >  pb = mkv->info.bc;
> >
> > -put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
> > +put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
> >  if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
> >  put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
> >  if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
> > @@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
> >  // after 4k and on a keyframe
> >  if (IS_SEEKABLE(pb, mkv)) {
> >  if (mkv->cluster_time_limit < 0)
> > -mkv->cluster_time_limit = 5000;
> > +mkv->cluster_time_limit = 5*(10/mkv->timecodescale);
> >  if (mkv->cluster_size_limit < 0)
> >  mkv->cluster_size_limit = 5 * 1024 * 1024;
> >  } else {
> >  if (mkv->cluster_time_limit < 0)
> > -mkv->cluster_time_limit = 1000;
> > +mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
> >  if (mkv->cluster_size_limit < 0)
> >  mkv->cluster_size_limit = 32 * 1024;
> >  }
> > @@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
> >  }
> >
> >  // ms precision is the de-facto standard timescale for mkv files
> > -avpriv_set_pts_info(st, 64, 1, 1000);
> > +avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);
> >
> >  if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
> >  if (mkv->mode == MODE_WEBM) {
> > @@ -2795,6 +2797,7 @@ static const AVOption options[] = {
> >  { "infer", "For each track type, mark the first track of disposition
> default as default; if none exists, mark the first track as default.", 0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS,
> "default_mode" },
> >  { "infer_no_subs", "For each track type, mark the first track of
> disposition default as default; for audio and video: if none exists, mark
> the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 =
> DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
> >  { "passthrough", "Use the disposition flag as-is", 0,
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS,
> "default_mode" },
> > +{ "timecodescale", "Time code scale for all tracks in nanoseconds",
> OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1,
> 10, FLAGS },
> >  { NULL },
> >  };
> >
>
> This, x1!
> Can we make it 1ns by default? Or maybe autodetect the highest precision
> timebase of all streams and use it?
>

The MKV spec seems to indicate a default value of ms. I did not want to
change it in this patch.


> Let's not keep generating garbage files with shitty rounding that players
> have to work around to recover the original timestamp based on framerate
> to reduce jitter (mpv does this!).
>

+1 on this, but I'm worry, we could break a lot of decoder which may not
follow the correct timecodescale (Which we found out in our system a while
back)


> Also WebM mode should disable this, because the spec is dumb and specifies
> a 1ms precision for no good reason.
>

If we keep the default value of ms, then this will not change any webm
unless a user decides to generate a Webm file and set this flag.
Do you want me to log an error saying that using this flag to set something
other than ms for webm is incorrect?


> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



-- 

Thierry Foucu
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread James Almer

On 1/13/2021 2:46 PM, Thierry Foucu wrote:

By default the time code scale in a MKV file in millisecond. With this
option we can set the time code scale to microsecond or nanoseconds for
very high frame rate.
---
  libavformat/matroskaenc.c | 11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 233c472b8f..cfad6a4693 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
  int default_mode;
  
  uint32_tsegment_uid[4];

+
+int64_t timecodescale;
  } MatroskaMuxContext;
  
  /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,

@@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
  return ret;
  pb = mkv->info.bc;
  
-put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);

+put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
  if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
  put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
  if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
@@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
  // after 4k and on a keyframe
  if (IS_SEEKABLE(pb, mkv)) {
  if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 5000;
+mkv->cluster_time_limit = 5*(10/mkv->timecodescale);


Either 10LL, or use av_rescale(). Same for the two below


  if (mkv->cluster_size_limit < 0)
  mkv->cluster_size_limit = 5 * 1024 * 1024;
  } else {
  if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 1000;
+mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
  if (mkv->cluster_size_limit < 0)
  mkv->cluster_size_limit = 32 * 1024;
  }
@@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
  }
  
  // ms precision is the de-facto standard timescale for mkv files

-avpriv_set_pts_info(st, 64, 1, 1000);
+avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);
  
  if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {

  if (mkv->mode == MODE_WEBM) {
@@ -2795,6 +2797,7 @@ static const AVOption options[] = {
  { "infer", "For each track type, mark the first track of disposition default as default; 
if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 
0, 0, FLAGS, "default_mode" },
  { "infer_no_subs", "For each track type, mark the first track of disposition default as 
default; for audio and video: if none exists, mark the first track as default.", 0, AV_OPT_TYPE_CONST, { 
.i64 = DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
  { "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST, { .i64 = 
DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, "default_mode" },
+{ "timecodescale", "Time code scale for all tracks in nanoseconds", 
OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1, 10, FLAGS },
  { NULL },
  };


Does this cover all cases? A timecodescale of 100 was implied until 
now, so many parts of the code could be hardcoding it in a non obvious way.


I see for example MATROSKA_ID_DURATION is set as av_rescale(s->duration, 
1000, AV_TIME_BASE). Should that AV_TIME_BASE be changed to 
mkv->timecodescale? The spec says "Duration of the Segment in 
nanoseconds based on TimestampScale."

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread Lynne
Jan 13, 2021, 18:46 by tfo...@gmail.com:

> By default the time code scale in a MKV file in millisecond. With this
> option we can set the time code scale to microsecond or nanoseconds for
> very high frame rate.
> ---
>  libavformat/matroskaenc.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 233c472b8f..cfad6a4693 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
>  int default_mode;
>  
>  uint32_tsegment_uid[4];
> +
> +int64_t timecodescale;
>  } MatroskaMuxContext;
>  
>  /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
> @@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
>  return ret;
>  pb = mkv->info.bc;
>  
> -put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
> +put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
>  if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
>  put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
>  if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
> @@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
>  // after 4k and on a keyframe
>  if (IS_SEEKABLE(pb, mkv)) {
>  if (mkv->cluster_time_limit < 0)
> -mkv->cluster_time_limit = 5000;
> +mkv->cluster_time_limit = 5*(10/mkv->timecodescale);
>  if (mkv->cluster_size_limit < 0)
>  mkv->cluster_size_limit = 5 * 1024 * 1024;
>  } else {
>  if (mkv->cluster_time_limit < 0)
> -mkv->cluster_time_limit = 1000;
> +mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
>  if (mkv->cluster_size_limit < 0)
>  mkv->cluster_size_limit = 32 * 1024;
>  }
> @@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
>  }
>  
>  // ms precision is the de-facto standard timescale for mkv files
> -avpriv_set_pts_info(st, 64, 1, 1000);
> +avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);
>  
>  if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
>  if (mkv->mode == MODE_WEBM) {
> @@ -2795,6 +2797,7 @@ static const AVOption options[] = {
>  { "infer", "For each track type, mark the first track of disposition default 
> as default; if none exists, mark the first track as default.", 0, 
> AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS, "default_mode" 
> },
>  { "infer_no_subs", "For each track type, mark the first track of disposition 
> default as default; for audio and video: if none exists, mark the first track 
> as default.", 0, AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER_NO_SUBS }, 0, 
> 0, FLAGS, "default_mode" },
>  { "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST, { 
> .i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, "default_mode" },
> +{ "timecodescale", "Time code scale for all tracks in nanoseconds", 
> OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1, 10, 
> FLAGS },
>  { NULL },
>  }; 
>

This, x1!
Can we make it 1ns by default? Or maybe autodetect the highest precision
timebase of all streams and use it?
Let's not keep generating garbage files with shitty rounding that players
have to work around to recover the original timestamp based on framerate
to reduce jitter (mpv does this!).
Also WebM mode should disable this, because the spec is dumb and specifies
a 1ms precision for no good reason.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] libavformat/matroskaenc.c: Add option to set timecodescale

2021-01-13 Thread Thierry Foucu
By default the time code scale in a MKV file in millisecond. With this
option we can set the time code scale to microsecond or nanoseconds for
very high frame rate.
---
 libavformat/matroskaenc.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 233c472b8f..cfad6a4693 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -158,6 +158,8 @@ typedef struct MatroskaMuxContext {
 int default_mode;
 
 uint32_tsegment_uid[4];
+
+int64_t timecodescale;
 } MatroskaMuxContext;
 
 /** 2 bytes * 7 for EBML IDs, 7 1-byte EBML lengths, 6 1-byte uint,
@@ -1827,7 +1829,7 @@ static int mkv_write_header(AVFormatContext *s)
 return ret;
 pb = mkv->info.bc;
 
-put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, 100);
+put_ebml_uint(pb, MATROSKA_ID_TIMECODESCALE, mkv->timecodescale);
 if ((tag = av_dict_get(s->metadata, "title", NULL, 0)))
 put_ebml_string(pb, MATROSKA_ID_TITLE, tag->value);
 if (!(s->flags & AVFMT_FLAG_BITEXACT)) {
@@ -1927,12 +1929,12 @@ static int mkv_write_header(AVFormatContext *s)
 // after 4k and on a keyframe
 if (IS_SEEKABLE(pb, mkv)) {
 if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 5000;
+mkv->cluster_time_limit = 5*(10/mkv->timecodescale);
 if (mkv->cluster_size_limit < 0)
 mkv->cluster_size_limit = 5 * 1024 * 1024;
 } else {
 if (mkv->cluster_time_limit < 0)
-mkv->cluster_time_limit = 1000;
+mkv->cluster_time_limit = 1*(10/mkv->timecodescale);
 if (mkv->cluster_size_limit < 0)
 mkv->cluster_size_limit = 32 * 1024;
 }
@@ -2708,7 +2710,7 @@ static int mkv_init(struct AVFormatContext *s)
 }
 
 // ms precision is the de-facto standard timescale for mkv files
-avpriv_set_pts_info(st, 64, 1, 1000);
+avpriv_set_pts_info(st, 64, 1, 10/mkv->timecodescale);
 
 if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT) {
 if (mkv->mode == MODE_WEBM) {
@@ -2795,6 +2797,7 @@ static const AVOption options[] = {
 { "infer", "For each track type, mark the first track of disposition 
default as default; if none exists, mark the first track as default.", 0, 
AV_OPT_TYPE_CONST, { .i64 = DEFAULT_MODE_INFER }, 0, 0, FLAGS, "default_mode" },
 { "infer_no_subs", "For each track type, mark the first track of 
disposition default as default; for audio and video: if none exists, mark the 
first track as default.", 0, AV_OPT_TYPE_CONST, { .i64 = 
DEFAULT_MODE_INFER_NO_SUBS }, 0, 0, FLAGS, "default_mode" },
 { "passthrough", "Use the disposition flag as-is", 0, AV_OPT_TYPE_CONST, { 
.i64 = DEFAULT_MODE_PASSTHROUGH }, 0, 0, FLAGS, "default_mode" },
+{ "timecodescale", "Time code scale for all tracks in nanoseconds", 
OFFSET(timecodescale), AV_OPT_TYPE_INT64, { .i64 = 100 }, 1, 10, 
FLAGS },
 { NULL },
 };
 
-- 
2.29.2.684.gfbc64c5ab5-goog

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/3] avutil/{avstring, bprint}: add XML escaping from ffprobe to avutil

2021-01-13 Thread Jan Ekström
On Wed, Jan 13, 2021 at 3:02 PM Nicolas George  wrote:
>
> Jan Ekström (12021-01-11):
> > From: Stefano Sabatini 
> >
> > ---
> >  libavutil/avstring.h |  1 +
> >  libavutil/bprint.c   | 14 ++
> >  tools/ffescape.c |  1 +
> >  3 files changed, 16 insertions(+)
>
> Please see
> https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/273500.html

Hi,

Yea, I actually noticed that one, but never got to responding to it.
Sorry about that.

While I do agree that such modes could be useful, currently I have no
need for the additional modes and thus the original code by Stefano
was just moved in avutil (and then utilized from ttmlenc). I did check
the XML specification that this code originally seemed to follow the
requirements (and thus mentioned the exact spot in the spec where this
is mentioned).

Thus, is the lack of those additional XML modes a blocker? Or should
they be added as the need arises?

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] Development of a CUDA accelerated variant of the libav vf_tonemap

2021-01-13 Thread Felix LeClair
I've pulled the branch and built with --enable-vulkan 
--enable-libglslang.


What else is needed? Do I need to pull the libplacebo repo as well 
and/or add any special enables in ./configure?


On Wed, Jan 13, 2021 at 5:12 am, Lynne  wrote:
Jan 12, 2021, 22:13 by felix.leclair...@hotmail.com 
:


 That's great! Any way for me to pull that branch or otherwise 
contribute?



The branch is here for now - 
The only blocker to having it merged is for me to rewrite the vulkan 
synchronization
mechanism we currently use. Which I should hopefully get around to 
soon.


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org 


To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org 
 with subject "unsubscribe".


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavu: support arbitrary-point FFT and all even (i)MDCT transforms

2021-01-13 Thread James Almer

On 1/12/2021 4:18 AM, Lynne wrote:

This patch adds support for arbitrary-point FFTs and all even MDCT
transforms.
Odd MDCTs are not supported yet as they're based on the DCT-II and DCT-III
and they're very niche.

With this we can now write tests.

Patch attached.


[...]


@@ -575,11 +651,13 @@ int TX_NAME(ff_tx_init_mdct_fft)(AVTXContext *s, av_tx_fn 
*tx,
  const void *scale, uint64_t flags)
 {
 const int is_mdct = ff_tx_type_is_mdct(type);
-int err, n = 1, m = 1, max_ptwo = 1 << (FF_ARRAY_ELEMS(fft_dispatch) - 1);
+int err, l, n = 1, m = 1, max_ptwo = 1 << (FF_ARRAY_ELEMS(fft_dispatch) - 
1);
 
 if (is_mdct)

 len >>= 1;
 
+l = len;

+
 #define CHECK_FACTOR(DST, FACTOR, SRC) 
\
 if (DST == 1 && !(SRC % FACTOR)) { 
\
 DST = FACTOR;  
\
@@ -601,12 +679,23 @@ int TX_NAME(ff_tx_init_mdct_fft)(AVTXContext *s, av_tx_fn 
*tx,
 s->inv = inv;
 s->type = type;
 
-/* Filter out direct 3, 5 and 15 transforms, too niche */

+/* If we weren't able to split the length into factors we can handle,
+ * resort to using the naive and slow FT. This also filters out
+ * direct 3, 5 and 15 transforms as they're too niche. */
 if (len > 1 || m == 1) {
-av_log(NULL, AV_LOG_ERROR, "Unsupported transform size: n = %i, "
-   "m = %i, residual = %i!\n", n, m, len);
-return AVERROR(EINVAL);
-} else if (n > 1 && m > 1) { /* 2D transform case */
+if (is_mdct && (l & 1)) /* Odd (i)MDCTs are not supported yet */
+return AVERROR(ENOTSUP);


I think ENOTSUP is not portable, which is why we use ENOSYS to signal 
unimplemented/unsupported features.



+s->n = l;
+s->m = 1;
+*tx = naive_fft;
+if (is_mdct) {
+s->scale = *((SCALE_TYPE *)scale);
+*tx = inv ? naive_imdct : naive_mdct;
+}
+return 0;
+}
+
+if (n > 1 && m > 1) { /* 2D transform case */
 if ((err = ff_tx_gen_compound_mapping(s)))
 return err;
 if (!(s->tmp = av_malloc(n*m*sizeof(*s->tmp
--
2.30.0.rc2


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavu: support arbitrary-point FFT and all even (i)MDCT transforms

2021-01-13 Thread Lynne
Jan 12, 2021, 08:18 by d...@lynne.ee:

> This patch adds support for arbitrary-point FFTs and all even MDCT 
> transforms.
> Odd MDCTs are not supported yet as they're based on the DCT-II and DCT-III
> and they're very niche.
>
> With this we can now write tests.
>
> Patch attached.
>
Pushed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] Patch for FFmpeg

2021-01-13 Thread Robin Cooksey
I’ve attached a patch which makes avformat handle the 308 Permanent Redirect 
HTTP status code – which is more recently defined in 
https://tools.ietf.org/html/rfc7538

The change just treats 308 in the same way as the other 30x status codes.








0001-Treat-HTTP-status-code-308-in-the-same-way-as-301-30.patch
Description: 0001-Treat-HTTP-status-code-308-in-the-same-way-as-301-30.patch
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avfilter: add estdif video filter

2021-01-13 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 doc/filters.texi |  64 ++
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/vf_estdif.c  | 469 +++
 4 files changed, 535 insertions(+)
 create mode 100644 libavfilter/vf_estdif.c

diff --git a/doc/filters.texi b/doc/filters.texi
index 813e35c2f9..9fbc4de7d2 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11065,6 +11065,70 @@ Flags to local 3x3 coordinates maps like this:
 
 This filter supports the all above options as @ref{commands}.
 
+@section estdif
+
+Deinterlace the input video ("estdif" stands for "Edge Slope
+Tracing Deinterlacing Filter").
+
+Spatial only filter that uses edge slope tracing algorithm
+to interpolate missing lines.
+It accepts the following parameters:
+
+@table @option
+@item mode
+The interlacing mode to adopt. It accepts one of the following values:
+
+@table @option
+@item frame
+Output one frame for each frame.
+@item field
+Output one frame for each field.
+@end table
+
+The default value is @code{field}.
+
+@item parity
+The picture field parity assumed for the input interlaced video. It accepts one
+of the following values:
+
+@table @option
+@item ff
+Assume the top field is first.
+@item bff
+Assume the bottom field is first.
+@item auto
+Enable automatic detection of field parity.
+@end table
+
+The default value is @code{auto}.
+If the interlacing is unknown or the decoder does not export this information,
+top field first will be assumed.
+
+@item deint
+Specify which frames to deinterlace. Accepts one of the following
+values:
+
+@table @option
+@item all
+Deinterlace all frames.
+@item interlaced
+Only deinterlace frames marked as interlaced.
+@end table
+
+The default value is @code{all}.
+
+@item rslope
+Specify the search radius for edge slope tracing. Default value is 1.
+Allowed range is from 1 to 15.
+
+@item redge
+Specify the search radius for best edge matching. Default value is 2.
+Allowed range is from 0 to 3.
+@end table
+
+@subsection Commands
+This filter supports same @ref{commands} as options.
+
 @section extractplanes
 
 Extract color channel components from input video stream into
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index ad1046d526..44afa79963 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -252,6 +252,7 @@ OBJS-$(CONFIG_EQ_FILTER) += vf_eq.o
 OBJS-$(CONFIG_EROSION_FILTER)+= vf_neighbor.o
 OBJS-$(CONFIG_EROSION_OPENCL_FILTER) += vf_neighbor_opencl.o opencl.o \
 opencl/neighbor.o
+OBJS-$(CONFIG_ESTDIF_FILTER) += vf_estdif.o
 OBJS-$(CONFIG_EXTRACTPLANES_FILTER)  += vf_extractplanes.o
 OBJS-$(CONFIG_FADE_FILTER)   += vf_fade.o
 OBJS-$(CONFIG_FFTDNOIZ_FILTER)   += vf_fftdnoiz.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index ce317dfa1c..471844a603 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -237,6 +237,7 @@ extern AVFilter ff_vf_entropy;
 extern AVFilter ff_vf_eq;
 extern AVFilter ff_vf_erosion;
 extern AVFilter ff_vf_erosion_opencl;
+extern AVFilter ff_vf_estdif;
 extern AVFilter ff_vf_extractplanes;
 extern AVFilter ff_vf_fade;
 extern AVFilter ff_vf_fftdnoiz;
diff --git a/libavfilter/vf_estdif.c b/libavfilter/vf_estdif.c
new file mode 100644
index 00..5d83e66906
--- /dev/null
+++ b/libavfilter/vf_estdif.c
@@ -0,0 +1,469 @@
+/*
+ * 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 "libavutil/common.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+#include "avfilter.h"
+#include "formats.h"
+#include "internal.h"
+#include "video.h"
+
+typedef struct ESTDIFContext {
+const AVClass *class;
+
+int mode; ///< 0 is frame, 1 is field
+int parity;   ///< frame field parity
+int deint;///< which frames to deinterlace
+int rslope;   ///< best edge slope search radius
+int redge;///< best edge match search radius
+int linesize[4];  ///< bytes of pixel data per line for each plane
+int planewidth[4];///< width of each plane
+int planeheight[4]; 

Re: [FFmpeg-devel] [PATCH] Add support for "omp simd" pragma.

2021-01-13 Thread Martin Storsjö

On Wed, 13 Jan 2021, Reimar Döffinger wrote:


If building with MSVC tools, yes you're right that armasm.exe/armasm64.exe 
takes a different syntax. But the gas-preprocessor tool (which is picked up 
automatically by our configure, one just needs to make sure it's available) 
handles expanding all the macros and rewriting directives into the armasm form, 
and feeding it to the armasm tools. Works fine and have done so for years. 
There's even a wiki page which tries to explain how to do it (although it's 
probably outdated in some aspects), see 
https://trac.ffmpeg.org/wiki/CompilationGuide/WinRT.



I went with the instructions in doc/platform.texi and that did not work at all,
It even tried to use cl.exe to compile the assembler files!


What did you end up trying/doing in this case? That sounds rather broken 
to me.


The main issue just is having gas-preprocessor available (but since you 
need a posix make, like from msys2, it should be pretty easy to have a 
perl installation for gas-preprocessor there) - but if it isn't found, 
configure really should be erroring out and not silently using cl as 
assembler...


My own setups for fate are a bit special as they're cross compiled from 
linux (with msvc wrapped in wine), but it should essentially just be 
"./configure --arch=arm64 --target-os=win32 --toolchain=msvc 
--enable-cross-compile", assuming you have MSVC targeting arm64 in $PATH.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add support for "omp simd" pragma.

2021-01-13 Thread Reimar Döffinger
> If building with MSVC tools, yes you're right that armasm.exe/armasm64.exe 
> takes a different syntax. But the gas-preprocessor tool (which is picked up 
> automatically by our configure, one just needs to make sure it's available) 
> handles expanding all the macros and rewriting directives into the armasm 
> form, and feeding it to the armasm tools. Works fine and have done so for 
> years. There's even a wiki page which tries to explain how to do it (although 
> it's probably outdated in some aspects), see 
> https://trac.ffmpeg.org/wiki/CompilationGuide/WinRT.
> 

I went with the instructions in doc/platform.texi and that did not work at all,
It even tried to use cl.exe to compile the assembler files!

> All of these run with full assembly optimizations enabled. So please don't 
> tell me that our assembly doesn't work on windows on arm, because it does, 
> and it has for years.
> 

My apologies. I’ll correct to: it doesn’t work using the instructions
shipping with the source code (as far as I can tell) :)

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/3] avutil/{avstring, bprint}: add XML escaping from ffprobe to avutil

2021-01-13 Thread Nicolas George
Jan Ekström (12021-01-11):
> From: Stefano Sabatini 
> 
> ---
>  libavutil/avstring.h |  1 +
>  libavutil/bprint.c   | 14 ++
>  tools/ffescape.c |  1 +
>  3 files changed, 16 insertions(+)

Please see
https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/273500.html

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 01/11] avcodec/vvc: add shared header for vvc

2021-01-13 Thread Nuo Mi
On Tue, Jan 12, 2021 at 6:01 AM Mark Thompson  wrote:

> On 10/01/2021 08:35, Nuo Mi wrote:
> > On Sun, Jan 10, 2021 at 9:39 AM Nuo Mi  wrote:
> >> On Sun, Jan 10, 2021 at 3:09 AM Mark Thompson  wrote:
> >>> On 09/01/2021 07:34, Nuo Mi wrote:
>  ---
> libavcodec/vvc.h | 124
> +++
> 1 file changed, 124 insertions(+)
> create mode 100644 libavcodec/vvc.h
> 
>  diff --git a/libavcodec/vvc.h b/libavcodec/vvc.h
>  new file mode 100644
>  index 00..0bd2acac1d
>  --- /dev/null
>  +++ b/libavcodec/vvc.h
>  @@ -0,0 +1,124 @@
>  ...
>  +
>  +// A.4.1: the highest level allows a MaxLumaPs of 35 651 584.
>  +VVC_MAX_LUMA_PS = 35651584,
>  +// A.4.1: pic_width_in_luma_samples and
> pic_height_in_luma_samples
> >>> are
>  +// constrained to be not greater than sqrt(MaxLumaPs * 8).  Hence
> >>> height/
>  +// width are bounded above by sqrt(8 * 35651584) = 16888.2
> samples.
>  +VVC_MAX_WIDTH  = 16888,
>  +VVC_MAX_HEIGHT = 16888,
>  +
>  +// A.4.1: table A.1 allows at most 440 tiles for any au.
>  +VVC_MAX_TILE_ROWS= 440,
> >>>
> >>> Is this bound really the best we can do?
> >>>
> >>> That is, is it actually possible to construct a valid stream with 440
> >>> tile rows?  It must have a single tile column and a height of at least
> >>> 14080 (for 440 rows of 32x32 CTUs), which feels extreme enough that it
> >>> might hit some of the other level constraints.
> >>>
> >> The  VVC_MAX_HEIGHT is 16888, it's higher than 14080.
> >> If we limit the VVC_MAX_HEIGHT to 4k, we can reduce it to 135.
>  >
>  > How about we define it as 20,  check the size and return error if > 20.
>  > 20 should enough for most of clips. hevc used 20.
>
> I'm specifically asking whether any of the other limits imply a better
> bound on the number of columns.  Can a 32x14080 stream (or something
> similar) with 440 tiles ever be valid given all of the constraints?
>
> 440 is not large enough that it would matter in terms of space used, so if
> there isn't actually a better implied limit then leave it as 440.  (<1kB -
> constrast that with the entry points, which are already >10kB of
> almost-never-used space with the current limit.)
>
No, I did not see and constraint in spec and the reference code. It does
not needed to be 32x14080, it can be 1920x14080 too.

>
>  +// A.4.1: table A.1 allows at most 20 tile columns for any level.
>  +VVC_MAX_TILE_COLUMNS = 20,
>  +
>  +// A.4.1 table A.1 allows at most 600 slice for any level.
>  +VVC_MAX_SLICES = 600,
>  ...
>
> - Mark
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] Fix for avformat DHAV parsing issue.

2021-01-13 Thread Idan Freiberg
Hello FFmpeg developers,

Iv'e noticed some DAV files containing a 0xff's padding in between DHAV
chunks. This is possible when the Dahua DVR device generates a DAV file
which is concatenated from 2 different DAV files kept on the device hard
drive.

Here  is a 30 seconds
sample DAV file with such concatenation. The current code (master) fails to
parse frame at 11:00:12 (clock on frame) as it incorrectly skips valid DHAV
chunks in the file after detecting the padding.

Proposing a possible patch for this issue (git formatted patch is attached).
Will like to hear your thoughts and insights.

Thank you

-- 
Idan Freiberg
Mobile: +972-52-2925213
From 89a7678883d061f3601b6260a2be5dc3471a76a9 Mon Sep 17 00:00:00 2001
From: Idan Freiberg 
Date: Wed, 13 Jan 2021 13:55:48 +0200
Subject: [PATCH] avformat/dhav: Fix incorrect non-DHAV chunk skipping logic

DAV files may contain a variable length padding in between chunks
filled with 0xff bytes. The current skipping logic is incorrect as it
may skip over DHAV chunks not appearing sequentially in the file.

We now look for the 'DHAV' tag using a byte-by-byte search in order
to handle such situations. Also the dhav->last_good_pos field will
not be updated while skipping unrecognized data.
---
 libavformat/dhav.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/libavformat/dhav.c b/libavformat/dhav.c
index 00e0d8476e..6a6c235e65 100644
--- a/libavformat/dhav.c
+++ b/libavformat/dhav.c
@@ -173,18 +173,9 @@ static int read_chunk(AVFormatContext *s)
 if (avio_feof(s->pb))
 return AVERROR_EOF;
 
-if (avio_rl32(s->pb) != MKTAG('D','H','A','V') && dhav->last_good_pos < INT64_MAX - 0x8000) {
-dhav->last_good_pos += 0x8000;
-avio_seek(s->pb, dhav->last_good_pos, SEEK_SET);
-
-while (avio_rl32(s->pb) != MKTAG('D','H','A','V')) {
-if (avio_feof(s->pb) || dhav->last_good_pos >= INT64_MAX - 0x8000)
-return AVERROR_EOF;
-dhav->last_good_pos += 0x8000;
-ret = avio_skip(s->pb, 0x8000 - 4);
-if (ret < 0)
-return ret;
-}
+while (avio_r8(s->pb) != 'D' || avio_r8(s->pb) != 'H' || avio_r8(s->pb) != 'A' || avio_r8(s->pb) != 'V') {
+if (avio_feof(s->pb))
+return AVERROR_EOF;
 }
 
 start = avio_tell(s->pb) - 4;
-- 
2.29.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/dhav: Fix incorrect non-DHAV chunk skipping logic

2021-01-13 Thread Idan Freiberg
DAV files may contain a variable length padding in between chunks
filled with 0xff bytes. The current skipping logic is incorrect as it
may skip over DHAV chunks not appearing sequentially in the file.

We now look for the 'DHAV' tag using a byte-by-byte search in order
to handle such situations. Also the dhav->last_good_pos field will
not be updated while skipping unrecognized data.
---
 libavformat/dhav.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/libavformat/dhav.c b/libavformat/dhav.c
index 00e0d8476e..6a6c235e65 100644
--- a/libavformat/dhav.c
+++ b/libavformat/dhav.c
@@ -173,18 +173,9 @@ static int read_chunk(AVFormatContext *s)
 if (avio_feof(s->pb))
 return AVERROR_EOF;
 
-if (avio_rl32(s->pb) != MKTAG('D','H','A','V') && dhav->last_good_pos < 
INT64_MAX - 0x8000) {
-dhav->last_good_pos += 0x8000;
-avio_seek(s->pb, dhav->last_good_pos, SEEK_SET);
-
-while (avio_rl32(s->pb) != MKTAG('D','H','A','V')) {
-if (avio_feof(s->pb) || dhav->last_good_pos >= INT64_MAX - 0x8000)
-return AVERROR_EOF;
-dhav->last_good_pos += 0x8000;
-ret = avio_skip(s->pb, 0x8000 - 4);
-if (ret < 0)
-return ret;
-}
+while (avio_r8(s->pb) != 'D' || avio_r8(s->pb) != 'H' || avio_r8(s->pb) != 
'A' || avio_r8(s->pb) != 'V') {
+if (avio_feof(s->pb))
+return AVERROR_EOF;
 }
 
 start = avio_tell(s->pb) - 4;
-- 
2.30.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavcodec/hevcdsp: port SIMD idct functions from 32-bit.

2021-01-13 Thread Martin Storsjö

On Tue, 12 Jan 2021, Reimar Döffinger wrote:


On 12 Jan 2021, at 13:24, Josh Dekker  wrote:

Hi,

AS  libavcodec/aarch64/hevcdsp_idct_neon.o
libavcodec/aarch64/hevcdsp_idct_neon.S: Assembler messages:
libavcodec/aarch64/hevcdsp_idct_neon.S:418: Error: operand mismatch -- `mov 
v29.4S,v28.4S'


Yes, I noticed that a few days ago, I sent the fixed version now. I had 
only tested on Apple assembler, assuming it would be the same. Really 
stupid behaviour by the GNU one, as if the type mattered for a mov 
instruction, needlessly complicates macros.


Yes, this particular restriction is a bit annoying, but there's also a 
number of other cases, with assembly constructs that are accepted by GAS 
but not supported by LLVM/Clang, like "umov w0, v0.4h[0]" (where the 
canonical form, that LLVM supports, is "umov w0, v0.h[0]").


If you just test your code with one tool, there's always a possibility of 
these slipping in, but by testing with more than one tool (either 
proactively by the developer, or by a reviewer, or ultimately by FATE) one 
can pick up on such issues, and in practice, all common tools (GAS, LLVM, 
and armasm64) usually support the canonical spellings of instructions.


(See my other reply about explanations around the fact that armasm64, 
while it uses a different syntax for directives, still can build all of 
our assembly thanks to the gas-preprocessor tool.)


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add support for "omp simd" pragma.

2021-01-13 Thread Martin Storsjö

Hi,

On Tue, 12 Jan 2021, Reimar Döffinger wrote:


I’m not sure it will discourage it more than having to write
the optimizations over and over, for Armv7 NEON, for Armv8 Linux,
for Armv8 Windows, then SVE/SVE2, who knows maybe Armv9
will also need a rewrite.


NEON code for armv8 windows and armv8 linux all use the exact same 
source, no need to write it twice.



For example macOS we are lucky enough that the assembler etc. are
largely compatible to Linux.


I'm not sure I'd say it's luck, it's pretty much by design there.

Historically, macOS build tools used an ancient fork of GAS, with very 
limited macroing capabilities. To remedy this, the gas-preprocessor tool 
was invented, for expanding modern gas macros, producing just a straight 
up feed of instructions, passed on to the native platform tools.


In modern times, the build tools are based on Clang/LLVM, and they support 
essentially all modern gas macro features (including altmacro, which was 
added in Clang 5). There's many parties that have an interest in this 
feature, e.g. support for building the Linux kernel with Clang.


Due to backwards compatibility with the old GAS fork's macroing capability 
(I think), there's some very vague differences between LLVM's macro 
support for other platforms and darwin, e.g. on other platforms, it's ok 
to invoke a macro as either "mymacro param1, param2" or "mymacro param1 
param2" (without commas between the arguments). On darwin targets, only 
the former works as intended.


All other platform differences are abstracted away with our macros in 
libavutil/aarch64/asm.S, see e.g. 
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavutil/aarch64/asm.S;h=d1fa72b3c65a4a58e76029e94b998d935649aa90;hb=ca21cb1e36ccae2ee71d4299d477fa9284c1f551#l85.
For darwin platforms, the movrel macro expands to "add rX, rX, 
symbol@PAGEOFF" while it expands to "add rX, rX, :lo12:symbol" on other 
platforms (ELF and COFF).


All the source files just use the high level macros function, endfunc, 
movrel, etc, which handle the few platform specific details that differ.



If you write code with just one tool, it's of course certainly possible to 
accidentally use some corner case detail that another tool objects to, but 
that's why one needs testing on multiple platforms, via a CI system or 
FATE or whatever. Just like you regularly need to test C code on various 
platforms, even if you'd expect it to work if it's properly written.



But for Windows-on-Arm there is no GNU assembler, and the Microsoft
assembler needs a completely different syntax, so even the assembly
we DO have just doesn’t work.


This is not true at all.

GCC and binutils don't support windows on arm/arm64 at all, that far is 
true.


But Clang/LLVM do (with https://github.com/mstorsjo/llvm-mingw you have an 
easily available packaged cross compiler and all), and they support the 
GAS syntax asm just fine.


If building with MSVC tools, yes you're right that armasm.exe/armasm64.exe 
takes a different syntax. But the gas-preprocessor tool (which is picked 
up automatically by our configure, one just needs to make sure it's 
available) handles expanding all the macros and rewriting directives into 
the armasm form, and feeding it to the armasm tools. Works fine and have 
done so for years. There's even a wiki page which tries to explain how to 
do it (although it's probably outdated in some aspects), see 
https://trac.ffmpeg.org/wiki/CompilationGuide/WinRT.


We even have regular fate tests of these configurations, see e.g. these:

http://fate.ffmpeg.org/report.cgi?slot=aarch64-mingw32-clang-trunk=20210113064430

http://fate.ffmpeg.org/report.cgi?time=20210109152105=arm64-msvc2019

http://fate.ffmpeg.org/report.cgi?slot=armv7-mingw32-clang-trunk=20210113055653

http://fate.ffmpeg.org/report.cgi?time=20210109163844=arm-msvc2019-phone

All of these run with full assembly optimizations enabled. So please don't 
tell me that our assembly doesn't work on windows on arm, because it does, 
and it has for years.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".