Re: [FFmpeg-devel] ABI break in 4.3
On Sunday 2020-07-05 01:04, James Almer wrote:
>On 7/4/2020 7:54 PM, Jan Engelhardt wrote:
>> @@ -67,6 +67,10 @@ typedef struct AVDCT {
>> +
>> +void (*get_pixels_unaligned)(int16_t *block /* align 16 */,
>> + const uint8_t *pixels,
>> + ptrdiff_t line_size);
>> } AVDCT;
>
>Neither of these are breaks as sizeof(AVCodecContext) and sizeof(AVDCT)
>are not part of the ABI. Both structs are meant to be allocated using
>avcodec_alloc_context3() and avcodec_dct_alloc() respectively, and not
>stored on stack or allocated with av_malloc(sizeof()).
There is sometimes a disconnect between how an API is meant to be
used, and how 3rd party programs actually use it. In the spirit of
making it "hard(er) to misuse", perhaps the struct definition should be
removed from the public header and instead be written as
struct AVDCT;
typedef struct AVDCT AVDCT;
so as to rule out av_malloc(sizeof(AVDCT)).
>[[the header file says:
> * You can use AVOptions (av_opt* / av_set/get*()) to access these fields from
> user
> * applications.]]
A "can" can be read as "need not". Perhaps that is what we are seeing with
blender which seems to access av fields directly, and has no noticable
av_set_ calls.
writeffmpeg.c: if ((of->oformat->flags & AVFMT_GLOBALHEADER)) {
writeffmpeg.c: if (of->oformat->flags & AVFMT_GLOBALHEADER) {
writeffmpeg.c: of->oformat = fmt;
writeffmpeg.c:of->packet_size = rd->ffcodecdata.mux_packet_size;
writeffmpeg.c: of->max_delay = (int)(0.7 * AV_TIME_BASE);
writeffmpeg.c: BLI_strncpy(of->filename, name, sizeof(of->filename));
writeffmpeg.c:if (avio_open(&of->pb, name, AVIO_FLAG_WRITE) < 0) {
writeffmpeg.c:&of->metadata, context->stamp_data,
ffmpeg_add_metadata_callback, false);
writeffmpeg.c: if (of->pb) {
writeffmpeg.c:avio_close(of->pb);
Or, summarized: A program may have been built with 4.3 but is combined
with 4.2.3 at runtime, then this can happen:
a = avcodec_dct_alloc(); // allocates 896
#ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED
a->get_pixels_unaligned = ffunc; // boom accessing byte ~952
#endif
If the API should not be used this way, it should not offer this way.
___
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] ABI break in 4.3
On Sun, Jul 5, 2020 at 10:43 AM Jan Engelhardt wrote: > > Or, summarized: A program may have been built with 4.3 but is combined > with 4.2.3 at runtime, then this can happen: > Running against an older version then the build version is never supported. ABI compatibility is only guaranteed forwards. An entirely new function could be added, and not touch existing ABI, and this scenario would still break. A distribution should never allow this to happen. - Hendrik ___ 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] ABI break in 4.3
sön 2020-07-05 klockan 10:43 +0200 skrev Jan Engelhardt:
> On Sunday 2020-07-05 01:04, James Almer wrote:
> > [[the header file says:
> > * You can use AVOptions (av_opt* / av_set/get*()) to access these fields
> > from user
> > * applications.]]
>
> A "can" can be read as "need not". Perhaps that is what we are seeing with
> blender which seems to access av fields directly, and has no noticable
> av_set_ calls.
>
> writeffmpeg.c: if ((of->oformat->flags & AVFMT_GLOBALHEADER)) {
> writeffmpeg.c: if (of->oformat->flags & AVFMT_GLOBALHEADER) {
> writeffmpeg.c: of->oformat = fmt;
> writeffmpeg.c:of->packet_size = rd->ffcodecdata.mux_packet_size;
> writeffmpeg.c: of->max_delay = (int)(0.7 * AV_TIME_BASE);
> writeffmpeg.c: BLI_strncpy(of->filename, name, sizeof(of->filename));
> writeffmpeg.c:if (avio_open(&of->pb, name, AVIO_FLAG_WRITE) < 0) {
> writeffmpeg.c:&of->metadata, context->stamp_data,
> ffmpeg_add_metadata_callback, false);
> writeffmpeg.c: if (of->pb) {
> writeffmpeg.c:avio_close(of->pb);
This looks like perfectly legitimate code to me. But, if Blender is
using libav*'s APIs incorrectly somewhere then Blender should be fixed.
> Or, summarized: A program may have been built with 4.3 but is combined
> with 4.2.3 at runtime, then this can happen:
>
> a = avcodec_dct_alloc(); // allocates 896
> #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED
> a->get_pixels_unaligned = ffunc; // boom accessing byte ~952
> #endif
"Doctor it hurts when I do this!"
Downgrading to a .so file with a lower minor version number than the
program is built against can never be expected to work. Else we
couldn't add new functions without a major bump.
/Tomas
___
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] Project orientation
sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > wrote: > > [...] > > > At some point, the project needs to decide what is in and what is > > > out, and since FFmpeg has numerous APIs, people can plug them > > > externally. It's not a problem to say "No" to a feature, > > > especially when, the number of people using that feature is under > > > 0,01% of FFmpeg users, and when people can build that externally > > > anyway. > > > > what we need is not to say "no" but to say > > "use this API, implement the module in your own repository, and > > register your module there" > > Once again, Michael, we agree, on this topic. > > No, means "not in the ffmpeg repo" does not mean "no, this is not > useful". I'd like to express a general agreement with this point. The project has experienced quite a lot of scope creep lately. We don't have to accommodate everyone, especially with the finite number of developers available. We can encourage people to maintain their own forks of FFmpeg, see for example FFmbc. /Tomas ___ 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] Project orientation
On Sun, 5 Jul 2020, Tomas Härdin wrote: sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > wrote: > [...] > > At some point, the project needs to decide what is in and what is > > out, and since FFmpeg has numerous APIs, people can plug them > > externally. It's not a problem to say "No" to a feature, > > especially when, the number of people using that feature is under > > 0,01% of FFmpeg users, and when people can build that externally > > anyway. > > what we need is not to say "no" but to say > "use this API, implement the module in your own repository, and > register your module there" Once again, Michael, we agree, on this topic. No, means "not in the ffmpeg repo" does not mean "no, this is not useful". I'd like to express a general agreement with this point. The project has experienced quite a lot of scope creep lately. We don't have to accommodate everyone, especially with the finite number of developers available. We can encourage people to maintain their own forks of FFmpeg, see for example FFmbc. And see how dead that project is. Or ffserver. Or x262. Regards, 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".
[FFmpeg-devel] How to resolve "ERROR: sdl2 requested but not found" when cross compile?
I did compile SDL2 first, and run command "sudo ln -s
/home/lixin/SDL2/bin/sdl2-config /usr/bin/arm-linux-gnueabihf-sdl2-config", but
I got "ERROR: sdl2 requested but not found" message when I did cross compile.
Here are commands what I did run for cross compile listed below:
# export CROSS_COMPILE=/usr/bin/arm-linux-gnueabihf-
# export CC=${CROSS_COMPILE}gcc
# ln -s /root/build/SDL2/bin/sdl2-config
/usr/bin/arm-linux-gnueabihf-sdl2-config
# ./configure --prefix=/root/build/ffmpeg --cross-prefix=$CROSS_COMPILE
--enable-gpl --enable-version3 --enable-nonfree --enable-cross-compile
--arch=arm --target-os=linux --cross-prefix=$CROSS_COMPILE --enable-pthreads
--disable-doc --disable-debug --disable-x86asm --disable-static --enable-shared
--enable-sdl2
___
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] ABI break in 4.3
On Sunday 2020-07-05 12:06, Tomas Härdin wrote: > >> Or, summarized: A program may have been built with 4.3 but is combined >> with 4.2.3 at runtime, then this can happen: >> >> a = avcodec_dct_alloc(); // allocates 896 >> #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED >> a->get_pixels_unaligned = ffunc; // boom accessing byte ~952 >> #endif > >"Doctor it hurts when I do this!" The application of this saying is short-sighted. It confounds basic exercise of features with overuse of said features. For lack of a better analogy, moving a leg normally _ought_ not to hurt in healthy humans. What is seen as "normal" is indeed situation-dependent, but the only other way to look at it is that ffmpeg is a disabled entity with special needs. >Downgrading to a .so file with a lower minor version number than the >program is built against can never be expected to work. Else we >couldn't add new functions without a major bump. Then you are doing it wrong. If one tries to run a contemporary program on an older distribution, which certainly has an older glibc, it refuses to run - which is much preferable to a crash at an arbitrary point down the line. It requires the use ELF symbol versions -- which ffmpeg fails to do properly. Between 4.2.3 and 4.3, avpriv_mpeg4audio_get_config2@@LIBAVCODEC_58 which is wrong. It should have been avpriv_mpeg4audio_get_config2@@LIBAVCODEC_58.91 Then the runtime linker ld-linux.so would have caught the problem at startup, because then, a program built with 4.3 would have a minimum requirement on elfsymver "58.91", and *not just* "58". ___ 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] Project orientation
sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > > wrote: > > > > [...] > > > > > At some point, the project needs to decide what is in and what is > > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > > externally. It's not a problem to say "No" to a feature, > > > > > especially when, the number of people using that feature is under > > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > > anyway. > > > > > > > > what we need is not to say "no" but to say > > > > "use this API, implement the module in your own repository, and > > > > register your module there" > > > > > > Once again, Michael, we agree, on this topic. > > > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > > useful". > > > > I'd like to express a general agreement with this point. The project > > has experienced quite a lot of scope creep lately. We don't have to > > accommodate everyone, especially with the finite number of developers > > available. > > > > We can encourage people to maintain their own forks of FFmpeg, see for > > example FFmbc. > > And see how dead that project is. Or ffserver. Or x262. That may be because Baptiste is focusing on other things. The code is still there. It still compiles. Would you want something experimental like x262 to be part of the FFmpeg codebase, for us to have to maintain forever? If you don't limit scope then that is what would happen. /Tomas ___ 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 v4] avdevice/xcbgrab: Add select_region option
This patch adds a select_region option to the xcbgrab input device.
If set to 1, the user will be prompted to select the grabbing area
graphically by clicking and dragging. A rectangle will be drawn to
mark the grabbing area. A single click with no dragging will select
the whole screen. The option overwrites the video_size, grab_x, and
grab_y options if set by the user.
For testing, just set the select_region option as follows:
ffmpeg -f x11grab -select_region 1 -i :0.0 output.mp4
The drawing happens directly on the root window using standard rubber
banding techniques, so it is very efficient and doesn't depend on any
X extensions or compositors.
Signed-off-by: Omar Emara
---
doc/indevs.texi | 7 +++
libavdevice/xcbgrab.c | 129 ++
2 files changed, 136 insertions(+)
diff --git a/doc/indevs.texi b/doc/indevs.texi
index 6f5afaf344..b5df111801 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -1478,6 +1478,13 @@ ffmpeg -f x11grab -framerate 25 -video_size cif -i
:0.0+10,20 out.mpg
@subsection Options
@table @option
+@item select_region
+Specify whether to select the grabbing area graphically using the pointer.
+A value of @code{1} prompts the user to select the grabbing area graphically
+by clicking and dragging. A single click with no dragging will select the
+whole screen. This option overwrites the @var{video_size}, @var{grab_x},
+and @var{grab_y} options. Default value is @code{0}.
+
@item draw_mouse
Specify whether to draw the mouse pointer. A value of @code{0} specifies
not to draw the pointer. Default value is @code{1}.
diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index 6f6b2dbf15..c5adb33b00 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -22,6 +22,7 @@
#include "config.h"
#include
+#include
#include
#if CONFIG_LIBXCB_XFIXES
@@ -69,6 +70,7 @@ typedef struct XCBGrabContext {
int show_region;
int region_border;
int centered;
+int select_region;
const char *framerate;
@@ -92,6 +94,7 @@ static const AVOption options[] = {
{ "centered", "Keep the mouse pointer at the center of grabbing region
when following.", 0, AV_OPT_TYPE_CONST, { .i64 = -1 }, INT_MIN, INT_MAX, D,
"follow_mouse" },
{ "show_region", "Show the grabbing region.", OFFSET(show_region),
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, D },
{ "region_border", "Set the region border thickness.",
OFFSET(region_border), AV_OPT_TYPE_INT, { .i64 = 3 }, 1, 128, D },
+{ "select_region", "Select the grabbing region graphically using the
pointer.", OFFSET(select_region), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
{ NULL },
};
@@ -668,6 +671,124 @@ static void setup_window(AVFormatContext *s)
draw_rectangle(s);
}
+#define CROSSHAIR_CURSOR 34
+
+static xcb_rectangle_t rectangle_from_corners(xcb_point_t *corner_a,
+ xcb_point_t *corner_b)
+{
+xcb_rectangle_t rectangle;
+rectangle.x = FFMIN(corner_a->x, corner_b->x);
+rectangle.y = FFMIN(corner_a->y, corner_b->y);
+rectangle.width = FFABS(corner_a->x - corner_b->x);
+rectangle.height = FFABS(corner_a->y - corner_b->y);
+return rectangle;
+}
+
+static int select_region(AVFormatContext *s)
+{
+XCBGrabContext *c = s->priv_data;
+xcb_connection_t *connection = c->conn;
+xcb_screen_t *screen = c->screen;
+
+int ret = 0;
+int done = 0;
+int was_pressed = 0;
+xcb_cursor_t cursor;
+xcb_font_t cursor_font;
+xcb_point_t press_position;
+xcb_generic_event_t *event;
+xcb_rectangle_t rectangle = {0};
+xcb_grab_pointer_reply_t *reply;
+xcb_grab_pointer_cookie_t cookie;
+
+xcb_window_t root_window = screen->root;
+xcb_gcontext_t gc = xcb_generate_id(connection);
+uint32_t mask = XCB_GC_FUNCTION | XCB_GC_SUBWINDOW_MODE;
+uint32_t values[] = {XCB_GX_INVERT,
+ XCB_SUBWINDOW_MODE_INCLUDE_INFERIORS};
+xcb_create_gc(connection, gc, root_window, mask, values);
+
+cursor_font = xcb_generate_id(connection);
+xcb_open_font(connection, cursor_font, strlen("cursor"), "cursor");
+cursor = xcb_generate_id(connection);
+xcb_create_glyph_cursor(connection, cursor, cursor_font, cursor_font,
+CROSSHAIR_CURSOR, CROSSHAIR_CURSOR + 1, 0, 0, 0,
+0x, 0x, 0x);
+cookie = xcb_grab_pointer(connection, 0, root_window,
+ XCB_EVENT_MASK_BUTTON_PRESS |
+ XCB_EVENT_MASK_BUTTON_RELEASE |
+ XCB_EVENT_MASK_BUTTON_MOTION,
+ XCB_GRAB_MODE_ASYNC, XCB_GRAB_MODE_ASYNC,
+ root_window, cursor, XCB_CURRENT_TIME);
+
+if (!(reply = xcb_grab_pointer_reply(connection, cookie, NULL)) ||
+(reply->status != XCB_GRAB_STATUS_SUCCESS)) {
+av_log(s, AV_LOG_ERROR,
+ "Failed to sel
[FFmpeg-devel] [PATCH] [GSoC 3/6] avformat/hls: use abr to switch streams
From: spartazhc
When abr is enable, it will take over the task to call http to
download segments, and will return a switch-request for hls to
switch streams.
For reason not to waste segments that have been downloaded,
switch will become effective after old segments is used out.
Abr cannot work with http_persistent option, and currently use
http_multiple.
Signed-off-by: spartazhc
---
doc/demuxers.texi | 3 +
libavformat/hls.c | 222 --
2 files changed, 218 insertions(+), 7 deletions(-)
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 3c15ab9eee..4cdbd95962 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -321,6 +321,9 @@ available in a metadata key named "variant_bitrate".
It accepts the following options:
@table @option
+@item abr
+enable abr to switch streams.
+
@item live_start_index
segment index to start live streams at (negative values are from the end).
diff --git a/libavformat/hls.c b/libavformat/hls.c
index 3798bdd5d1..b8f202e6a8 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -189,6 +189,15 @@ struct variant {
char subtitles_group[MAX_FIELD_LEN];
};
+struct throughput {
+int n_throughputs;
+
+/* throughputs are in kbps, always record the last 20 times */
+float throughput_fifo[20];
+int head;
+int tail;
+};
+
typedef struct HLSContext {
AVClass *class;
AVFormatContext *ctx;
@@ -213,8 +222,36 @@ typedef struct HLSContext {
int http_multiple;
int http_seekable;
AVIOContext *playlist_pb;
+
+int abr;
+struct throughput *throughputs;
+int can_switch;
+int switch_request2;
+int switch_delay;
+int64_t switch_timestamp;
+int64_t delta_timestamp;
+int cur_pls;
} HLSContext;
+static struct segment *next_segment(struct playlist *pls);
+static int open_input(HLSContext *c, struct playlist *pls, struct segment
*seg, AVIOContext **in);
+
+static void sync_cur_seq(HLSContext *c) {
+int i;
+for (i = 0; i < c->n_playlists; i++) {
+struct playlist *pls = c->playlists[i];
+pls->cur_seq_no = c->cur_seq_no;
+}
+}
+
+static struct segment *next2_segment(struct playlist *pls)
+{
+int n = pls->cur_seq_no - pls->start_seq_no + 2;
+if (n >= pls->n_segments)
+return NULL;
+return pls->segments[n];
+}
+
static void free_segment_dynarray(struct segment **segments, int n_segments)
{
int i;
@@ -624,6 +661,33 @@ static int open_url_keepalive(AVFormatContext *s,
AVIOContext **pb,
#endif
}
+static int update_throughputs(struct throughput *thr, float time, int pb_size)
+{
+if (pb_size <= 0 || time <= 0)
+return -1;
+if (thr->n_throughputs < 20) {
+++thr->n_throughputs;
+thr->throughput_fifo[thr->tail] = (float)(pb_size) / time;
+} else {
+thr->throughput_fifo[thr->tail] = (float)(pb_size) / time;
+++thr->head;
+}
+
+thr->tail = (thr->tail + 1) % 20;
+return 0;
+}
+
+static int64_t get_switch_timestamp(HLSContext *c, struct playlist *pls)
+{
+int64_t pos = c->first_timestamp == AV_NOPTS_VALUE ?
+ 0 : c->first_timestamp;
+
+for (int i = 0; i < pls->cur_seq_no + 2; i++) {
+pos += pls->segments[i]->duration;
+}
+return pos;
+}
+
static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
AVDictionary *opts, AVDictionary *opts2, int *is_http_out)
{
@@ -633,7 +697,10 @@ static int open_url(AVFormatContext *s, AVIOContext **pb,
const char *url,
int ret;
int is_http = 0;
-if (av_strstart(url, "crypto", NULL)) {
+if (av_strstart(url, "abr", NULL)) {
+if (url[3] == '+' || url[3] == ':')
+proto_name = avio_find_protocol_name(url + 4);
+} else if (av_strstart(url, "crypto", NULL)) {
if (url[6] == '+' || url[6] == ':')
proto_name = avio_find_protocol_name(url + 7);
} else if (av_strstart(url, "data", NULL)) {
@@ -665,6 +732,8 @@ static int open_url(AVFormatContext *s, AVIOContext **pb,
const char *url,
if (!strncmp(proto_name, url, strlen(proto_name)) &&
url[strlen(proto_name)] == ':')
;
+else if (av_strstart(url, "abr", NULL) && !strncmp(proto_name, url + 4,
strlen(proto_name)) && url[4 + strlen(proto_name)] == ':')
+;
else if (av_strstart(url, "crypto", NULL) && !strncmp(proto_name, url + 7,
strlen(proto_name)) && url[7 + strlen(proto_name)] == ':')
;
else if (av_strstart(url, "data", NULL) && !strncmp(proto_name, url + 5,
strlen(proto_name)) && url[5 + strlen(proto_name)] == ':')
@@ -690,6 +759,31 @@ static int open_url(AVFormatContext *s, AVIOContext **pb,
const char *url,
} else {
ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
}
+if (c->abr) {
+AVDictionary *abr_ret = NULL;
+AVDictionaryEntry *en = NULL;
+struct segment *seg;
+int pb_size, switch_request;
+av_opt_get_dict_val(*
[FFmpeg-devel] [PATCH] [GSoC 2/6] avformat/http: Add abr to whitelist
From: spartazhc
add abr protocol to http's whitelist
Signed-off-by: spartazhc
---
libavformat/http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/http.c b/libavformat/http.c
index 6c39da1a8b..6a58c9afef 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1815,7 +1815,7 @@ const URLProtocol ff_http_protocol = {
.priv_data_size = sizeof(HTTPContext),
.priv_data_class = &http_context_class,
.flags = URL_PROTOCOL_FLAG_NETWORK,
-.default_whitelist = "http,https,tls,rtp,tcp,udp,crypto,httpproxy,data"
+.default_whitelist =
"http,https,tls,rtp,tcp,udp,crypto,httpproxy,data,abr"
};
#endif /* CONFIG_HTTP_PROTOCOL */
--
2.27.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".
[FFmpeg-devel] [PATCH] [GSoC 1/6] avformat/abr: Adaptive Bitrate support
From: spartazhc
Add abr module for hls/dash.
Signed-off-by: spartazhc
---
doc/protocols.texi | 7 +
libavformat/Makefile| 1 +
libavformat/abr.c | 282
libavformat/protocols.c | 1 +
4 files changed, 291 insertions(+)
create mode 100644 libavformat/abr.c
diff --git a/doc/protocols.texi b/doc/protocols.texi
index 644a17963d..fc80209884 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -51,6 +51,13 @@ in microseconds.
A description of the currently available protocols follows.
+@section abr
+
+Adaptive bitrate sub-protocol work for hls/dash.
+
+The abr protocol takes stream information from hls/dash as input,
+use bandwidth estimation to decide whether to switch or not.
+
@section amqp
Advanced Message Queueing Protocol (AMQP) version 0-9-1 is a broker based
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 26af859a28..3c08289d5e 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -588,6 +588,7 @@ OBJS-$(CONFIG_LIBOPENMPT_DEMUXER)+= libopenmpt.o
OBJS-$(CONFIG_VAPOURSYNTH_DEMUXER) += vapoursynth.o
# protocols I/O
+OBJS-$(CONFIG_ABR_PROTOCOL) += abr.o
OBJS-$(CONFIG_ASYNC_PROTOCOL)+= async.o
OBJS-$(CONFIG_APPLEHTTP_PROTOCOL)+= hlsproto.o
OBJS-$(CONFIG_BLURAY_PROTOCOL) += bluray.o
diff --git a/libavformat/abr.c b/libavformat/abr.c
new file mode 100644
index 00..b7d00efcae
--- /dev/null
+++ b/libavformat/abr.c
@@ -0,0 +1,282 @@
+/*
+ * Adaptive Bitrate Module for HLS / DASH
+ * Copyright (c) 2020 Hongcheng Zhong
+ *
+ * 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 "avformat.h"
+#include "libavutil/opt.h"
+#include "libavutil/time.h"
+#include "libavutil/avstring.h"
+#include "url.h"
+#include
+
+enum ABRFormatType {
+ABR_TYPE_HLS,
+ABR_TYPE_DASH
+};
+
+typedef struct variant_bitrate {
+int value;
+int index;
+} variant_bitrate;
+
+typedef struct ABRContext {
+AVClass *class;
+URLContext *hd;
+AVDictionary *abr_params;
+AVDictionary *abr_metadata;
+enum ABRFormatType format;
+int cur_pls;
+int can_switch;
+int n_variants;
+variant_bitrate *variants_bitrate;
+int index;
+int n_throughputs;
+float *throughputs;
+int64_t position;
+} ABRContext;
+
+static float harmonic_mean(int num, float* arr)
+{
+float tmp = 0;
+
+if (num <= 0) return 0;
+
+for (size_t i = 0; i < num; i++) {
+tmp += 1 / arr[i];
+}
+
+return num / tmp;
+}
+
+static int hls_param_parse(ABRContext *c, const char *key, const char *value)
+{
+if (!av_strcasecmp(key, "cur_pls")) {
+c->cur_pls = atoi(value);
+} else if (!av_strcasecmp(key, "can_switch")) {
+c->can_switch = atoi(value);
+} else if (!av_strcasecmp(key, "n_variants")) {
+c->n_variants = atoi(value);
+c->variants_bitrate = av_mallocz(sizeof(variant_bitrate) *
c->n_variants);
+if (!c->variants_bitrate)
+return -1;
+} else if (av_strstart(key, "variant_bitrate", NULL)) {
+c->variants_bitrate[c->index].value = atoi(value);
+c->variants_bitrate[c->index].index = c->index;
+c->index++;
+} else if (!av_strcasecmp(key, "n_throughputs")) {
+c->n_throughputs = atoi(value);
+c->index = 0;
+if (c->n_throughputs > 0) {
+c->throughputs = av_malloc(sizeof(float) * c->n_throughputs);
+if (!c->throughputs)
+return -1;
+}
+} else if (av_strstart(key, "throughputs", NULL))
+c->throughputs[c->index++] = atof(value);
+return 0;
+}
+
+static int dash_param_parse(ABRContext *c, const char *key, const char *value)
+{
+return 0;
+}
+
+static int abr_param_parse(ABRContext *c, enum ABRFormatType type, const char
*key, const char *value)
+{
+if (type == ABR_TYPE_HLS) {
+hls_param_parse(c, key, value);
+} else if (type == ABR_TYPE_DASH) {
+dash_param_parse(c, key, value);
+}
+return 0;
+}
+
+static int compare_vb(const void *a, const void *b)
+{
+variant_bitrate *a1 = (variant_bitrate *)a;
+variant_bitrate *a2 = (variant_bitrate *)b;
+return (*a2).value - (*a1).value;
+}
+
+static int ab
[FFmpeg-devel] [PATCH] [GSoC 5/6] avformat/utils: add av_packet_clean to remove AVPackets not needed in pktl
From: spartazhc
Add av_packet_clean to remove AVPackets whose stream_index is not
in st_index list.
Generally s->internal->packet_buffer may have pkts from different
stream, and stream_index will be used to discard pkt that is not
needed. But in case of abr, several streams may pass the stream_index
check. So we need a function to remove AVPackets not needed in pktl
added by hls_read_header.
Signed-off-by: spartazhc
---
libavformat/avformat.h | 9 +++
libavformat/internal.h | 15 +++
libavformat/utils.c| 59 ++
libavformat/version.h | 2 +-
4 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index e91e7f1d33..90dee0d075 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -2474,6 +2474,15 @@ int avformat_seek_file(AVFormatContext *s, int
stream_index, int64_t min_ts, int
*/
int avformat_flush(AVFormatContext *s);
+/**
+ * Remove the AVPackets do not needed in the packet list.
+ * The behaviour is undefined if the packet list is empty.
+ *
+ * @param s media file handle
+ * @param st_index the stream_index list which is needed
+ */
+int av_packet_clean(AVFormatContext *s, int *st_index);
+
/**
* Start playing a network-based stream (e.g. RTSP stream) at the
* current position.
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 17a6ab07d3..ac943b1441 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -764,6 +764,21 @@ int ff_packet_list_put(AVPacketList **head, AVPacketList
**tail,
int ff_packet_list_get(AVPacketList **head, AVPacketList **tail,
AVPacket *pkt);
+/**
+ * Remove the AVPackets do not needed in the packet list.
+ * The behaviour is undefined if the packet list is empty.
+ *
+ * @note The pkt will be overwritten completely. The caller owns the
+ * packet and must unref it by itself.
+ *
+ * @param head List head element
+ * @param tail List tail element
+ * @param st_index the stream_index list which is needed
+ * @return 0 on success. Success is guaranteed
+ * if the packet list is not empty.
+ */
+int ff_packet_list_clean(AVPacketList **head, AVPacketList **tail,
+ int *st_index);
/**
* Wipe the list and unref all the packets in it.
*
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 45a4179552..26b5a08a19 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1565,6 +1565,65 @@ int ff_packet_list_get(AVPacketList **pkt_buffer,
return 0;
}
+/**
+ * return 1 if needed
+ */
+static int ff_check_st_index(int st, int *st_index)
+{
+for (int i = 0; i < AVMEDIA_TYPE_NB; ++i) {
+if (st_index[i] == st)
+return 1;
+}
+return 0;
+}
+
+int ff_packet_list_clean(AVPacketList **pkt_buffer,
+ AVPacketList **pkt_buffer_end,
+ int *st_index)
+{
+AVPacketList *pktl, *pktn;
+av_assert0(*pkt_buffer);
+pktl = *pkt_buffer;
+pktn = pktl->next;
+
+/* num >= 3 */
+while (pktn && pktn != *pkt_buffer_end) {
+if (!ff_check_st_index(pktn->pkt.stream_index, st_index)) {
+av_packet_unref(&pktn->pkt);
+pktl->next = pktn->next;
+av_freep(pktn);
+pktn = pktl->next;
+} else {
+pktl = pktn;
+pktn = pktn->next;
+}
+}
+/* last one*/
+if (pktn && !ff_check_st_index(pktn->pkt.stream_index, st_index)) {
+av_packet_unref(&pktn->pkt);
+av_freep(pktn);
+pktl->next = NULL;
+*pkt_buffer_end = pktl;
+}
+/* first one*/
+pktl = *pkt_buffer;
+if (!ff_check_st_index(pktl->pkt.stream_index, st_index)) {
+av_packet_unref(&pktl->pkt);
+*pkt_buffer = pktl->next;
+if (!pktl->next)
+*pkt_buffer_end = NULL;
+av_freep(&pktl);
+}
+
+return 0;
+}
+
+int av_packet_clean(AVFormatContext *s, int *st_index) {
+int ret = ff_packet_list_clean(&s->internal->packet_buffer,
+ &s->internal->packet_buffer_end, st_index);
+return ret;
+}
+
static int64_t ts_to_samples(AVStream *st, int64_t ts)
{
return av_rescale(ts, st->time_base.num * st->codecpar->sample_rate,
st->time_base.den);
diff --git a/libavformat/version.h b/libavformat/version.h
index 3c1957b00c..75c03fde0a 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
// Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
// Also please add any ticket numbers that you believe might be affected here
#define LIBAVFORMAT_VERSION_MAJOR 58
-#define LIBAVFORMAT_VERSION_MINOR 47
+#define LIBAVFORMAT_VERSION_MINOR 48
#define LIBAVFORMAT_VERSION_MICRO 100
#define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
--
2.27.0
___
ffmpeg-devel mailing list
ffmpeg-
[FFmpeg-devel] [PATCH] [GSoC 4/6] ffplay: add an option to enable abr
From: spartazhc
Add abr option, ffplay can play hls using abr by:
ffplay -i http://xxx/master.m3u8 -abr
Structure ABRList is added to save stream type and index, it is
used to allow packet_queue_put function to put pkt which from same
type(for example: video pkt) but different stream index to queue.
Signed-off-by: spartazhc
---
doc/ffplay.texi | 2 +
fftools/ffplay.c | 145 ---
2 files changed, 138 insertions(+), 9 deletions(-)
diff --git a/doc/ffplay.texi b/doc/ffplay.texi
index f3761bb12e..6a24542cda 100644
--- a/doc/ffplay.texi
+++ b/doc/ffplay.texi
@@ -46,6 +46,8 @@ Disable audio.
Disable video.
@item -sn
Disable subtitles.
+@item -abr
+Enable adaptive bitrate for hls/dash.
@item -ss @var{pos}
Seek to @var{pos}. Note that in most formats it is not possible to seek
exactly, so @command{ffplay} will seek to the nearest seek point to
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index d673b8049a..b17b75fa8f 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -201,6 +201,15 @@ typedef struct Decoder {
SDL_Thread *decoder_tid;
} Decoder;
+typedef struct ABRList {
+int **audio_list;
+int audios;
+int **video_list;
+int videos;
+int **sub_list;
+int subs;
+} ABRList;
+
typedef struct VideoState {
SDL_Thread *read_tid;
AVInputFormat *iformat;
@@ -305,6 +314,8 @@ typedef struct VideoState {
int last_video_stream, last_audio_stream, last_subtitle_stream;
SDL_cond *continue_read_thread;
+
+ABRList *abr_list;
} VideoState;
/* options specified by the user */
@@ -356,6 +367,7 @@ static char *afilters = NULL;
static int autorotate = 1;
static int find_stream_info = 1;
static int filter_nbthreads = 0;
+static int abr = 0;
/* current context */
static int is_full_screen;
@@ -1262,6 +1274,29 @@ static void stream_component_close(VideoState *is, int
stream_index)
}
}
+static void free_abr_dynarray(int **list, int num)
+{
+for (int i = 0; i < num; i++) {
+av_free(list[i]);
+}
+}
+
+static void free_abr_list(ABRList *abrlist)
+{
+if (abrlist->audios) {
+free_abr_dynarray(abrlist->audio_list, abrlist->audios);
+av_freep(&abrlist->audio_list);
+}
+if (abrlist->videos) {
+free_abr_dynarray(abrlist->video_list, abrlist->videos);
+av_freep(&abrlist->video_list);
+}
+if (abrlist->subs) {
+free_abr_dynarray(abrlist->sub_list, abrlist->subs);
+av_freep(&abrlist->sub_list);
+}
+}
+
static void stream_close(VideoState *is)
{
/* XXX: use a special url_shutdown call to abort parse cleanly */
@@ -2753,6 +2788,67 @@ static int is_realtime(AVFormatContext *s)
return 0;
}
+static av_cold int abr_init_list(VideoState *is)
+{
+int stream_index, *tmp;
+AVStream *st;
+int nb_streams = is->ic->nb_streams;
+ABRList *abrlist = is->abr_list;
+
+for (stream_index = 0; stream_index < nb_streams; stream_index++) {
+st = is->ic->streams[stream_index];
+tmp = av_memdup(&stream_index, sizeof(int));
+if (!tmp)
+return -1;
+switch (st->codecpar->codec_type) {
+case AVMEDIA_TYPE_AUDIO:
+av_dynarray_add(&abrlist->audio_list, &abrlist->audios, tmp);
+break;
+case AVMEDIA_TYPE_VIDEO:
+av_dynarray_add(&abrlist->video_list, &abrlist->videos, tmp);
+break;
+case AVMEDIA_TYPE_SUBTITLE:
+av_dynarray_add(&abrlist->sub_list, &abrlist->subs, tmp);
+break;
+default:
+av_free(tmp);
+break;
+}
+}
+return 0;
+}
+
+static int abr_check_list(ABRList *abr_list, enum AVMediaType type, int st)
+{
+int **st_list;
+int n_st;
+switch (type) {
+case AVMEDIA_TYPE_AUDIO:
+st_list = abr_list->audio_list;
+n_st = abr_list->audios;
+break;
+case AVMEDIA_TYPE_VIDEO:
+st_list = abr_list->video_list;
+n_st = abr_list->videos;
+break;
+case AVMEDIA_TYPE_SUBTITLE:
+st_list = abr_list->sub_list;
+n_st = abr_list->subs;
+break;
+default:
+break;
+}
+if (!st_list)
+return 0;
+for (int i = 0; i < n_st; i++) {
+if (*st_list[i] == st)
+return 1;
+}
+return 0;
+}
+
+
+
/* this thread gets the stream from the disk or the network */
static int read_thread(void *arg)
{
@@ -2789,6 +2885,8 @@ static int read_thread(void *arg)
av_dict_set(&format_opts, "scan_all_pmts", "1",
AV_DICT_DONT_OVERWRITE);
scan_all_pmts_set = 1;
}
+if (abr)
+av_dict_set(&format_opts, "abr", "1", 0);
err = avformat_open_input(&ic, is->filename, is->iformat, &format_opts);
if (err < 0) {
print_error(is->filename, err);
@@ -2918,6 +3016,15 @@ static int read_thread(vo
[FFmpeg-devel] [PATCH] [GSoC 6/6] ffplay: add av_packet_clean to remove AVPackets not needed
From: spartazhc
hls_read_header will add all streams to s->internal->packet_buffer.
Use av_packet_clean to remove the AVPackets from other streams that
are not needed, otherwise abr will allow them to be added to ffplay's
packet_queue.
Signed-off-by: spartazhc
---
fftools/ffplay.c | 4
1 file changed, 4 insertions(+)
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index b17b75fa8f..019dc7f32e 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -3018,6 +3018,10 @@ static int read_thread(void *arg)
/* clean packet list filled in hls_read_header if abr is enabled */
if (abr) {
+ret = av_packet_clean(ic, st_index);
+if (ret < 0) {
+av_log(NULL, AV_LOG_WARNING, "Failed to clean av_packet\n");
+}
is->abr_list = av_mallocz(sizeof(ABRList));
ret = abr_init_list(is);
if (ret < 0) {
--
2.27.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] ABI break in 4.3
sön 2020-07-05 klockan 12:54 +0200 skrev Jan Engelhardt: > On Sunday 2020-07-05 12:06, Tomas Härdin wrote: > > > Or, summarized: A program may have been built with 4.3 but is > > > combined > > > with 4.2.3 at runtime, then this can happen: > > > > > > a = avcodec_dct_alloc(); // allocates 896 > > > #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED > > > a->get_pixels_unaligned = ffunc; // boom accessing byte ~952 > > > #endif > > > > "Doctor it hurts when I do this!" > > The application of this saying is short-sighted. It confounds basic > exercise of features with overuse of said features. For lack of a > better analogy, moving a leg normally _ought_ not to hurt in healthy > humans. What is seen as "normal" is indeed situation-dependent, but > the only other way to look at it is that ffmpeg is a disabled entity > with special needs. We can certainly change the API to make this sort of break harder, with a major bump. But given what the documentation says about the current API it is not a break. It might not be the most user-friendly API, but it is right there in the headers. There's quite a bit of inertia involved too. This makes me wonder if there's a way to prevent AVCodecContext from being allocated anywhere but the heap, without having it and similar structs just be opaque pointers on the user side. > > Downgrading to a .so file with a lower minor version number than > > the > > program is built against can never be expected to work. Else we > > couldn't add new functions without a major bump. > > Then you are doing it wrong. If one tries to run a contemporary > program on an older distribution, which certainly has an older glibc, > it refuses to run - which is much preferable to a crash at an > arbitrary point down the line. > > It requires the use ELF symbol versions -- which ffmpeg fails to > do properly. Between 4.2.3 and 4.3, > > avpriv_mpeg4audio_get_config2@@LIBAVCODEC_58 > > which is wrong. It should have been > > avpriv_mpeg4audio_get_config2@@LIBAVCODEC_58.91 > > Then the runtime linker ld-linux.so would have caught the problem > at startup, because then, a program built with 4.3 would have a > minimum requirement on elfsymver "58.91", and *not just* "58". This is a fair point. I didn't actually know the loader can do stuff like this, sounds super handy. How hard would it be to get that going? /Tomas ___ 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] Project orientation
Quoting Tomas Härdin (2020-07-05 13:19:25) > sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: > > > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > > > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > > > wrote: > > > > > [...] > > > > > > At some point, the project needs to decide what is in and what is > > > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > > > externally. It's not a problem to say "No" to a feature, > > > > > > especially when, the number of people using that feature is under > > > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > > > anyway. > > > > > > > > > > what we need is not to say "no" but to say > > > > > "use this API, implement the module in your own repository, and > > > > > register your module there" > > > > > > > > Once again, Michael, we agree, on this topic. > > > > > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > > > useful". > > > > > > I'd like to express a general agreement with this point. The project > > > has experienced quite a lot of scope creep lately. We don't have to > > > accommodate everyone, especially with the finite number of developers > > > available. > > > > > > We can encourage people to maintain their own forks of FFmpeg, see for > > > example FFmbc. > > > > And see how dead that project is. Or ffserver. Or x262. > > That may be because Baptiste is focusing on other things. The code is > still there. It still compiles. > > Would you want something experimental like x262 to be part of the > FFmpeg codebase, for us to have to maintain forever? If you don't limit > scope then that is what would happen. I have been thinking about something like a staging branch. Or multiple such "experimental" branches in the main repository, which would still be "official" in some sense, even though they wouldn't be the master one. There obviously seems to be a conflict of visions between people who want ffmpeg to be a stable consistent coherent tool for our downstreams and those who want to play with experimental concepts, cute hacks and fringe features. I would count myself among the former, but the latter is also a valid worldview and should not be disregarded. I would like to hope some kind of a compromise can be found. Will write a more complete reply to OP later. -- Anton Khirnov ___ 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 v06 2/5] fbtile helperRoutines cpu based framebuffer detiling
Jul 5, 2020, 04:57 by hanish...@gmail.com: > Hi Lynne, > > > On Sun, 5 Jul, 2020, 00:53 Lynne, wrote: > >> Jul 4, 2020, 14:17 by hanish...@gmail.com: >> >> > Add helper routines which can be used to detile tiled framebuffer >> > layouts into a linear layout, using the cpu. >> > >> > Currently it supports Legacy Intel Tile-X, Legacy Intel Tile-Y and >> > Newer Intel Tile-Yf tiled layouts. >> > >> > Currently supported pixel format is 32bit RGB. >> > >> > It also contains detile_generic logic, which can be easily configured >> > to support different kinds of tiling layouts, at the expense of some >> > processing speed, compared to developing a targeted detiling logic. >> > >> >> Sorry, but we really cannot afford to have this as part of the public API, >> which it needs to be if it can be used by lavfi. So its unacceptable in its >> current form, especially if all parts dependent on it are compile-time >> options. >> That's why I suggested merging it all into hwcontext_drm.c >> > > > > There is no compile time options which it's users/callers need to > manipulate to use it. > > What I meant by easily configurable is, in future if we want to add support > for a new (currently unknown to it) tile layout, there is no need to do it > from scratch. One could just add a new set of boundry conditions with the > required direction changes, and the generic logic can take care of doing > the required tile walking. This is no different than say adding a new > colour format or bit resolution or so to a scaler for example, rather I > would say much simpler than these. > > And once the new definition is added into fbtile. It can be used by any of > its users without any additional change. > > That's how any library or for that matter any code works currently, so not > sure what's diffrent here that you are objecting to. > > Also tile manipulation is a logically independent set of operations which > can be used by any logic and not just hwcontext_drm, and tiling is nothing > unique to hwcontext_drm, so it will be better to keep it independent of > hwcontext_drm, because logically/technically it's independent of it. > > Also I seem to be missing something here, what is the technical/... reason > you don't want a frame buffer tile manipulation logic as a public API, in > case if it is? > Something like this, is like I said, very niche and optionally fixes behavior that shouldn't ever have been exposed had it not been for the incredibly bad decisions libdrm made (and we made as well). And for that, and for not fitting in as any part of any library, I don't want a public API for this anywhere. Something like this could belong in libswscale, but that requires not resorting to hacks and actually implementing tiled pixel formats, which we're definitely not doing. Hence any patches that actually fix the behavior non-optionally would be what's accepted. We're not looking for solutions outside of hwcontext_drm. Like I said, no other API is that badly designed to give the user access to tiled data. There' no point in having a generic filter/hwdownload setting because there's nothing other than hwcontext_drm that would use it properly. While I'm okay with having the detiling as a separate file, it cannot be exposed via the public API and any user wishing to detile would have to use the hwcontext API. And I don't want this as an optional filter either. And no, you can't just duplicate the tiling code to libavfilter. Bottom line is, we don't want command-line users to _ever_ see or touch tiled content, and to do that, you have to make hwcontext_drm.c detile when downloading. ___ 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] ABI break in 4.3
On Sunday 2020-07-05 13:39, Tomas Härdin wrote:
>>> Downgrading to a .so file with a lower minor version number than
>>> the program is built against can never be expected to work. Else
>>> we couldn't add new functions without a major bump.
>>
>> It requires the use ELF symbol versions -- which ffmpeg fails to
>> do properly.[...]
>
>This is a fair point. I didn't actually know the loader can do stuff
>like this, sounds super handy. How hard would it be to get that going?
Change libavcodec.v to
LIBAVCODEC_58 {
global:
av_foo;
av_bar;
av_whatever_else_there_is;...
local:
*;
};
LIBAVCODEC_58.91 {
global:
avpriv_mpeg4audio_get_config2;
} LIBAVCODEC_58;
LIBAVCODEC_58.123 { /* future example */
global:
avblahblah;
} LIBAVCODEC_58.91;
Repeat likewise for other .v files. The file is no longer a template. The
automatic substitution of "_MAJOR" by the build system needs to cease. Version
numbers in the file are supposed to be fixed (among the set of all .so files
that share a SONAME).
___
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/hlsenc: Remove duplicate close of the output stream.
Ping?
On 2020-07-01 17:59, Andrey Semashev wrote:
The result of the first close attempt is ignored and may be lost. By removing
it we ensure the close result code is properly analyzed.
---
libavformat/hlsenc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 71fa3db060..88b58a1ba8 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2631,7 +2631,6 @@ static int hls_write_trailer(struct AVFormatContext *s)
goto failed;
vs->size = range_length;
-hlsenc_io_close(s, &vs->out, filename);
ret = hlsenc_io_close(s, &vs->out, filename);
if (ret < 0) {
av_log(s, AV_LOG_WARNING, "upload segment failed, will retry with a
new http session.\n");
___
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] Project orientation
On Sun, 5 Jul 2020, Tomas Härdin wrote: sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: On Sun, 5 Jul 2020, Tomas Härdin wrote: > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > wrote: > > > [...] > > > > At some point, the project needs to decide what is in and what is > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > externally. It's not a problem to say "No" to a feature, > > > > especially when, the number of people using that feature is under > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > anyway. > > > > > > what we need is not to say "no" but to say > > > "use this API, implement the module in your own repository, and > > > register your module there" > > > > Once again, Michael, we agree, on this topic. > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > useful". > > I'd like to express a general agreement with this point. The project > has experienced quite a lot of scope creep lately. We don't have to > accommodate everyone, especially with the finite number of developers > available. > > We can encourage people to maintain their own forks of FFmpeg, see for > example FFmbc. And see how dead that project is. Or ffserver. Or x262. That may be because Baptiste is focusing on other things. The code is still there. It still compiles. That is the point. Baptise is focusing on other things, and the code he used to do rots in an external repository. I guess most of the features he implemented crept back to ffmpeg (I did the backporting of some features myself), but still, it was duplicate work. Forks are good, if you are submitting the code back to master. If not, then forks are not so good because they often dont't live on alone and you lose the features in it. Would you want something experimental like x262 to be part of the FFmpeg codebase, for us to have to maintain forever? If you don't limit scope then that is what would happen. x262 is another example of a fork, where the fork alone was not popular/interesting enough to live on. If it were merged to x264, I am fairly certain it would not be experimental anymore, and we'd have an MPEG2 encoder which would scale much better to multiple cores than what we have now in ffmpeg. So having something merged and maintained in ffmpeg has the benefit that more people have the ability to test the code and to develop it. Also it is more likely for the project to get developers if their code live in the core ffmpeg respository. I also don't see that maitenance burden to be so huge. And usefulness is also questionable. I think it is safe to say that having AMQP/ZMQ protocol support is much more useful then Lego Racers demuxer... (and I quite liked Lego Racers!!!). So my opinion would be to be inclusive when merging stuff. I am not saying everything has to be merged, I rejected not very long ago the NIT table parsing or the EPG "data decoder", these were really out of scope and more importantly out of the current capabilites of the framework. And if distros are worried about dependencies, they can always make two packages, a normal and a full. And I'd also like to point to the linux kernel as an example of a monolitic code repository which seems to work quite well. Regards, 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".
[FFmpeg-devel] [PATCH 1/3] avfilter/vf_subtitles: add shift option
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 8
libavfilter/vf_subtitles.c | 23 +--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..c962ac55b0 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17935,6 +17935,9 @@ The filter accepts the following options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17991,6 +17994,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour=&HCCFF'
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..125fbd9ac7 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -103,6 +104,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -267,6 +273,7 @@ static const AVOption subtitles_options[] = {
{"stream_index", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"si", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"force_style", "force subtitle style", OFFSET(force_style),
AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS},
+{"shift","shift subtitles timing", OFFSET(shift),
AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS },
{NULL},
};
@@ -297,7 +304,7 @@ AVFILTER_DEFINE_CLASS(subtitles);
static av_cold int init_subtitles(AVFilterContext *ctx)
{
-int j, ret, sid;
+int j, ret, sid, nskip;
int k = 0;
AVDictionary *codec_opts = NULL;
AVFormatContext *fmt = NULL;
@@ -448,6 +455,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_init_packet(&pkt);
pkt.data = NULL;
pkt.size = 0;
+nskip = 0;
while (av_read_frame(fmt, &pkt) >= 0) {
int i, got_subtitle;
AVSubtitle sub = {0};
@@ -458,8 +466,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n",
av_err2str(ret));
} else if (got_subtitle) {
-const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000));
+const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
const int64_t duration = sub.end_display_time;
+
+if (start_time + duration < 0) {
+nskip++;
+goto pkt_end;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time
range.\n", nskip);
+nskip = 0;
+}
+
for (i = 0; i < sub.num_rects; i++) {
char *ass_line = sub.rects[i]->ass;
if (!ass_line)
@@ -472,6 +489,8 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
}
}
}
+
+pkt_end:
av_packet_unref(&pkt);
avsubtitle_free(&sub);
}
--
2.17.1
___
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 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index c962ac55b0..c4ca39cb6d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,15 +17930,12 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
-@item shift
-Shift subtitles timings by the specified amount.
-
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17952,12 +17950,18 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@item shift
+Shift subtitles timings by the specified amount.
+@end table
+
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
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] [GSoC 5/6] avformat/utils: add av_packet_clean to remove AVPackets not needed in pktl
Hongcheng Zhong:
> From: spartazhc
>
> Add av_packet_clean to remove AVPackets whose stream_index is not
> in st_index list.
>
> Generally s->internal->packet_buffer may have pkts from different
> stream, and stream_index will be used to discard pkt that is not
> needed. But in case of abr, several streams may pass the stream_index
> check. So we need a function to remove AVPackets not needed in pktl
> added by hls_read_header.
>
> Signed-off-by: spartazhc
> ---
> libavformat/avformat.h | 9 +++
> libavformat/internal.h | 15 +++
> libavformat/utils.c| 59 ++
> libavformat/version.h | 2 +-
> 4 files changed, 84 insertions(+), 1 deletion(-)
>
First: I do not know whether we even need this API or not; but I
nevertheless looked at your proposal.
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index e91e7f1d33..90dee0d075 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2474,6 +2474,15 @@ int avformat_seek_file(AVFormatContext *s, int
> stream_index, int64_t min_ts, int
> */
> int avformat_flush(AVFormatContext *s);
>
> +/**
> + * Remove the AVPackets do not needed in the packet list.
> + * The behaviour is undefined if the packet list is empty.
> + *
This is completely wrong. It is even more wrong than for
ff_packet_list_clean, as the user has no way to check whether the packet
list is empty.
> + * @param s media file handle
> + * @param st_index the stream_index list which is needed
> + */
> +int av_packet_clean(AVFormatContext *s, int *st_index);
I don't like "clean" as this sounds as if the whole list were discarded;
something like "filter" sounds better.
> +
> /**
> * Start playing a network-based stream (e.g. RTSP stream) at the
> * current position.
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index 17a6ab07d3..ac943b1441 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -764,6 +764,21 @@ int ff_packet_list_put(AVPacketList **head, AVPacketList
> **tail,
> int ff_packet_list_get(AVPacketList **head, AVPacketList **tail,
> AVPacket *pkt);
>
> +/**
> + * Remove the AVPackets do not needed in the packet list.
Wrong English "do not needed"
> + * The behaviour is undefined if the packet list is empty.
> + *
Undefined if empty? That is very bad behaviour. If it is empty, then
there simply are no packets to discard and this function shouldn't do
anything.
> + * @note The pkt will be overwritten completely. The caller owns the
> + * packet and must unref it by itself.
> + *
You obviously copied this (and the undefined if empty) from
ff_packet_list_get. It makes no sense at all here.
> + * @param head List head element
> + * @param tail List tail element
> + * @param st_index the stream_index list which is needed
This list needs a length argument (or a documented sentinel). You are
currently hardcoding AVMEDIA_TYPE_NB in an undocumented manner and this
is wrong.
> + * @return 0 on success. Success is guaranteed
> + * if the packet list is not empty.
> + */
> +int ff_packet_list_clean(AVPacketList **head, AVPacketList **tail,
> + int *st_index);
> /**
> * Wipe the list and unref all the packets in it.
> *
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 45a4179552..26b5a08a19 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1565,6 +1565,65 @@ int ff_packet_list_get(AVPacketList **pkt_buffer,
> return 0;
> }
>
> +/**
> + * return 1 if needed
> + */
> +static int ff_check_st_index(int st, int *st_index)
> +{
> +for (int i = 0; i < AVMEDIA_TYPE_NB; ++i) {
> +if (st_index[i] == st)
> +return 1;
> +}
> +return 0;
> +}
> +
> +int ff_packet_list_clean(AVPacketList **pkt_buffer,
> + AVPacketList **pkt_buffer_end,
> + int *st_index)
> +{
> +AVPacketList *pktl, *pktn;
> +av_assert0(*pkt_buffer);
> +pktl = *pkt_buffer;
> +pktn = pktl->next;
> +
> +/* num >= 3 */
?
> +while (pktn && pktn != *pkt_buffer_end) {
> +if (!ff_check_st_index(pktn->pkt.stream_index, st_index)) {
> +av_packet_unref(&pktn->pkt);
> +pktl->next = pktn->next;
> +av_freep(pktn);
This does not free pktn (which leaks -- use valgrind to see it for
yourself). It will instead av_free pktn->pkt.buf which is NULL because
of the av_packet_unref above.
> +pktn = pktl->next;
> +} else {
> +pktl = pktn;
> +pktn = pktn->next;
> +}
> +}
> +/* last one*/
> +if (pktn && !ff_check_st_index(pktn->pkt.stream_index, st_index)) {
> +av_packet_unref(&pktn->pkt);
> +av_freep(pktn);
> +pktl->next = NULL;
> +*pkt_buffer_end = pktl;
> +}
Is handling the last one differently really necessary? Wouldn't it be
enough to remove pktn != *pkt
Re: [FFmpeg-devel] Project orientation
> > > Would you want something experimental like x262 to be part of the > > FFmpeg codebase, for us to have to maintain forever? If you don't limit > > scope then that is what would happen. > > x262 is another example of a fork, where the fork alone was not > popular/interesting enough to live on. If it were merged to x264, I am > fairly certain it would not be experimental anymore, and we'd have an > MPEG2 encoder which would scale much better to multiple cores than what we > have now in ffmpeg. > Highly unlikely, x264 development has essentially ground to a halt. And people use H.264 a lot still. x262 works well enough for an old format like MPEG-2. There's no real need to develop it unless someone needs to eke out an extra 2% compression because they have a million MPEG-2 receivers they can't change. The original x264 developers didn't want a merge back by the way. And I'd also like to point to the linux kernel as an example of a > monolitic code repository which seems to work quite well. > Exception that proves the rule. A lot of developers there are paid full time to work on the kernel and just do cleanup, merge old patches etc. Kieran ___ 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] Project orientation
> > So having something merged and maintained in ffmpeg has the benefit that > more people have the ability to test the code and to develop it. Also it > is more likely for the project to get developers if their code live in the > core ffmpeg respository. I also don't see that maitenance burden to be so > huge. And usefulness is also questionable. I think it is safe to say that > having AMQP/ZMQ protocol support is much more useful then Lego Racers > demuxer... (and I quite liked Lego Racers!!!). > Maybe also a full implementation of WebRTC? Or QUIC? The problem is that libavformat is not a network stack and adding more protocols means that it's harder and harder to build a real network stack. See the way UDP has a hacky thread built in to stop packet loss. Kieran ___ 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] lavc/cfhd:3d transform decoding for both progressive and interlaced
>
> > @@ -1064,6 +1446,6 @@ AVCodec ff_cfhd_decoder = {
> > .init = cfhd_init,
> > .close= cfhd_close,
> > .decode = cfhd_decode,
> > -.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
> > -.caps_internal= FF_CODEC_CAP_INIT_THREADSAFE |
> FF_CODEC_CAP_INIT_CLEANUP,
> > +.capabilities = AV_CODEC_CAP_DR1,
> > +.caps_internal= FF_CODEC_CAP_INIT_CLEANUP,
> > };
>
Dunno, I guess the student just grepped for THREAD and removed it.
I did not notice that.
Kieran
___
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] [GSoC 5/6] avformat/utils: add av_packet_clean to remove AVPackets not needed in pktl
- On Jul 5, 2020, at 9:14 PM, Andreas Rheinhardt
andreas.rheinha...@gmail.com wrote:
> Hongcheng Zhong:
>> From: spartazhc
>>
>> Add av_packet_clean to remove AVPackets whose stream_index is not
>> in st_index list.
>>
>> Generally s->internal->packet_buffer may have pkts from different
>> stream, and stream_index will be used to discard pkt that is not
>> needed. But in case of abr, several streams may pass the stream_index
>> check. So we need a function to remove AVPackets not needed in pktl
>> added by hls_read_header.
>>
>> Signed-off-by: spartazhc
>> ---
>> libavformat/avformat.h | 9 +++
>> libavformat/internal.h | 15 +++
>> libavformat/utils.c| 59 ++
>> libavformat/version.h | 2 +-
>> 4 files changed, 84 insertions(+), 1 deletion(-)
>>
>
> First: I do not know whether we even need this API or not; but I
> nevertheless looked at your proposal.
>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index e91e7f1d33..90dee0d075 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -2474,6 +2474,15 @@ int avformat_seek_file(AVFormatContext *s, int
>> stream_index, int64_t min_ts, int
>> */
>> int avformat_flush(AVFormatContext *s);
>>
>> +/**
>> + * Remove the AVPackets do not needed in the packet list.
>> + * The behaviour is undefined if the packet list is empty.
>> + *
>
> This is completely wrong. It is even more wrong than for
> ff_packet_list_clean, as the user has no way to check whether the packet
> list is empty.
>
>> + * @param s media file handle
>> + * @param st_index the stream_index list which is needed
>> + */
>> +int av_packet_clean(AVFormatContext *s, int *st_index);
>
> I don't like "clean" as this sounds as if the whole list were discarded;
> something like "filter" sounds better.
>
>> +
>> /**
>> * Start playing a network-based stream (e.g. RTSP stream) at the
>> * current position.
>> diff --git a/libavformat/internal.h b/libavformat/internal.h
>> index 17a6ab07d3..ac943b1441 100644
>> --- a/libavformat/internal.h
>> +++ b/libavformat/internal.h
>> @@ -764,6 +764,21 @@ int ff_packet_list_put(AVPacketList **head, AVPacketList
>> **tail,
>> int ff_packet_list_get(AVPacketList **head, AVPacketList **tail,
>> AVPacket *pkt);
>>
>> +/**
>> + * Remove the AVPackets do not needed in the packet list.
>
> Wrong English "do not needed"
>
>> + * The behaviour is undefined if the packet list is empty.
>> + *
>
> Undefined if empty? That is very bad behaviour. If it is empty, then
> there simply are no packets to discard and this function shouldn't do
> anything.
>
>> + * @note The pkt will be overwritten completely. The caller owns the
>> + * packet and must unref it by itself.
>> + *
>
> You obviously copied this (and the undefined if empty) from
> ff_packet_list_get. It makes no sense at all here.
>
>> + * @param head List head element
>> + * @param tail List tail element
>> + * @param st_index the stream_index list which is needed
>
> This list needs a length argument (or a documented sentinel). You are
> currently hardcoding AVMEDIA_TYPE_NB in an undocumented manner and this
> is wrong.
>
>> + * @return 0 on success. Success is guaranteed
>> + * if the packet list is not empty.
>> + */
>> +int ff_packet_list_clean(AVPacketList **head, AVPacketList **tail,
>> + int *st_index);
>> /**
>> * Wipe the list and unref all the packets in it.
>> *
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 45a4179552..26b5a08a19 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -1565,6 +1565,65 @@ int ff_packet_list_get(AVPacketList **pkt_buffer,
>> return 0;
>> }
>>
>> +/**
>> + * return 1 if needed
>> + */
>> +static int ff_check_st_index(int st, int *st_index)
>> +{
>> +for (int i = 0; i < AVMEDIA_TYPE_NB; ++i) {
>> +if (st_index[i] == st)
>> +return 1;
>> +}
>> +return 0;
>> +}
>> +
>> +int ff_packet_list_clean(AVPacketList **pkt_buffer,
>> + AVPacketList **pkt_buffer_end,
>> + int *st_index)
>> +{
>> +AVPacketList *pktl, *pktn;
>> +av_assert0(*pkt_buffer);
>> +pktl = *pkt_buffer;
>> +pktn = pktl->next;
>> +
>> +/* num >= 3 */
>
> ?
>
>> +while (pktn && pktn != *pkt_buffer_end) {
>> +if (!ff_check_st_index(pktn->pkt.stream_index, st_index)) {
>> +av_packet_unref(&pktn->pkt);
>> +pktl->next = pktn->next;
>> +av_freep(pktn);
>
> This does not free pktn (which leaks -- use valgrind to see it for
> yourself). It will instead av_free pktn->pkt.buf which is NULL because
> of the av_packet_unref above.
>
>> +pktn = pktl->next;
>> +} else {
>> +pktl = pktn;
>> +pktn = pktn->next;
>> +}
>> +}
>> +/* last one*/
>> +if (pktn && !ff_check_st_index(p
[FFmpeg-devel] patch submission questions
Hello, I'm trying to submit a patch for adding a "shift" option to subtitles/ass filters. Initial submission was ok, but resubmitting after addressing some emails didn't go as expected. I have the following two questions on the process: Q1: How do you get the "v2" label when you resubmit a patch? I thought this would happen auto-magically, but it didn't. And I couldn't find anything relevant in https://ffmpeg.org/developer.html#Contributing Q2: In a patchset consisting of several commits, is each commit expected to be "standalone"? I.e. does it have to apply cleanly without depending on the previous commits in the patchset? Again this is not mentioned in the contributing guidelines, but my 2/3 patch didn't make it to patchwork.ffmpeg.org, and this seems like a plausible explanation. Thanks in advance, Manolis ___ 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 submission questions
On Sun, 2020-07-05 at 15:42 +0200, Manolis Stamatogiannakis wrote: > Hello, > > I'm trying to submit a patch for adding a "shift" option to > subtitles/ass > filters. Initial submission was ok, but resubmitting after addressing > some > emails didn't go as expected. > > I have the following two questions on the process: > > Q1: How do you get the "v2" label when you resubmit a patch? I > thought this > would happen auto-magically, but it didn't. > And I couldn't find anything relevant in > https://ffmpeg.org/developer.html#Contributing > > Q2: In a patchset consisting of several commits, is each commit > expected to > be "standalone"? I.e. does it have to apply cleanly without depending > on > the previous commits in the patchset? > Again this is not mentioned in the contributing guidelines, but my > 2/3 > patch didn't make it to patchwork.ffmpeg.org, and this seems like a > plausible explanation. > > Thanks in advance, > Manolis > ___ > 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". You can use `git format-patch -v -n` to get patches like [PATCH v2 1/20]. See git-format-patch documentation for more details. ___ 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 2/3] avfilter/vf_subtitles: add shift option for ass filter
Previously option implemented only for subtitles filter.
Signed-off-by: Manolis Stamatogiannakis
---
libavfilter/vf_subtitles.c | 35 ---
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 125fbd9ac7..09cca6aab8 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -67,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -106,7 +107,7 @@ static av_cold int init(AVFilterContext *ctx)
if (ass->shift != 0) {
ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
-av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
}
ass->library = ass_library_init();
@@ -233,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass);
static av_cold int init_ass(AVFilterContext *ctx)
{
+int eid, nskip;
+ASS_Event *event;
AssContext *ass = ctx->priv;
int ret = init(ctx);
@@ -249,6 +252,24 @@ static av_cold int init_ass(AVFilterContext *ctx)
ass->filename);
return AVERROR(EINVAL);
}
+
+/* Shift subtitles. */
+nskip = 0;
+for (eid = 0; eid < ass->track->n_events; eid++) {
+event = &ass->track->events[eid];
+event->Start += ass->shift;
+if (event->Start + event->Duration < 0) {
+ass_free_event(ass->track, eid);
+nskip++;
+continue;
+} else if (nskip > 0) {
+av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time
range.\n", nskip);
+memmove(event - nskip, event, (ass->track->n_events - eid) *
sizeof(ASS_Event));
+ass->track->n_events -= nskip;
+nskip = 0;
+}
+}
+
return 0;
}
@@ -273,7 +294,6 @@ static const AVOption subtitles_options[] = {
{"stream_index", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"si", "set stream index", OFFSET(stream_index),
AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS},
{"force_style", "force subtitle style", OFFSET(force_style),
AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS},
-{"shift","shift subtitles timing", OFFSET(shift),
AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS },
{NULL},
};
@@ -466,6 +486,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx)
av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n",
av_err2str(ret));
} else if (got_subtitle) {
+/* Shift subtitles. */
const int64_t start_time = av_rescale_q(sub.pts,
AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
Re: [FFmpeg-devel] ABI break in 4.3
On 7/5/2020 5:43 AM, Jan Engelhardt wrote:
>
> On Sunday 2020-07-05 01:04, James Almer wrote:
>> On 7/4/2020 7:54 PM, Jan Engelhardt wrote:
>>> @@ -67,6 +67,10 @@ typedef struct AVDCT {
>>> +
>>> +void (*get_pixels_unaligned)(int16_t *block /* align 16 */,
>>> + const uint8_t *pixels,
>>> + ptrdiff_t line_size);
>>> } AVDCT;
>>
>> Neither of these are breaks as sizeof(AVCodecContext) and sizeof(AVDCT)
>> are not part of the ABI. Both structs are meant to be allocated using
>> avcodec_alloc_context3() and avcodec_dct_alloc() respectively, and not
>> stored on stack or allocated with av_malloc(sizeof()).
>
> There is sometimes a disconnect between how an API is meant to be
> used, and how 3rd party programs actually use it. In the spirit of
> making it "hard(er) to misuse", perhaps the struct definition should be
> removed from the public header and instead be written as
>
> struct AVDCT;
> typedef struct AVDCT AVDCT;
>
> so as to rule out av_malloc(sizeof(AVDCT)).
>
>
>> [[the header file says:
>> * You can use AVOptions (av_opt* / av_set/get*()) to access these fields
>> from user
>> * applications.]]
>
> A "can" can be read as "need not".
And you'd be correct. Direct access is allowed and offsets are
guaranteed within a soname version no matter the release.
> Perhaps that is what we are seeing with
> blender which seems to access av fields directly, and has no noticable
> av_set_ calls.
>
> writeffmpeg.c: if ((of->oformat->flags & AVFMT_GLOBALHEADER)) {
> writeffmpeg.c: if (of->oformat->flags & AVFMT_GLOBALHEADER) {
> writeffmpeg.c: of->oformat = fmt;
> writeffmpeg.c:of->packet_size = rd->ffcodecdata.mux_packet_size;
> writeffmpeg.c: of->max_delay = (int)(0.7 * AV_TIME_BASE);
> writeffmpeg.c: BLI_strncpy(of->filename, name, sizeof(of->filename));
> writeffmpeg.c:if (avio_open(&of->pb, name, AVIO_FLAG_WRITE) < 0) {
> writeffmpeg.c:&of->metadata, context->stamp_data,
> ffmpeg_add_metadata_callback, false);
> writeffmpeg.c: if (of->pb) {
> writeffmpeg.c:avio_close(of->pb);
As Thomas mentioned, this looks like valid usage of the API, so the
issue in Blender must be something else. All those AVFormatContext
fields can be accessed directly.
Unrelated to this, but they should stop using of->filename, for that
matter, and use of->url instead. The former has been deprecated for two
years and a half, and will be removed in an upcoming soname bump.
>
> Or, summarized: A program may have been built with 4.3 but is combined
> with 4.2.3 at runtime, then this can happen:
>
> a = avcodec_dct_alloc(); // allocates 896
> #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED
> a->get_pixels_unaligned = ffunc; // boom accessing byte ~952
> #endif
>
> If the API should not be used this way, it should not offer this way.
But it isn't offered that way. I don't know what part of avdct.h made
you think it could be used that way.
First, you're setting a pointer you're not meant to set yourself.
Nowhere in the doxy it says you can do that. It's not an user settable
callback. You're asked to initialize the struct with avcodec_dct_init().
Was it the best design choice? Probably not. Something like pixelutils.h
in libavutil may be a better approach, and AVDCT could be changed into
something like it.
And second, you're running a program built against 4.3 with a lavc 4.2.x
at runtime. That is not supported for obvious reasons and the source of
your "boom". You can (or should be able to) use 4.3 at runtime on a
program built against an ffmpeg release as old as 4.0, but no the
opposite way.
Your suggestion to change the way we generate the .ver files to outright
refuse to run in the above scenario is interesting, but may be quite a
lot of work considering the amount of public functions we export.
___
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 submission questions
On Sun, Jul 05, 2020 at 03:42:34PM +0200, Manolis Stamatogiannakis wrote: > Q2: In a patchset consisting of several commits, is each commit expected to > be "standalone"? I.e. does it have to apply cleanly without depending on > the previous commits in the patchset? No, but it has to compile and work even if not applying any of the latter patches. (This increases reviewability and bisectability.) AFAIK Patchwork doesn't enforce this, though. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ 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] ABI break in 4.3
Am So., 5. Juli 2020 um 10:43 Uhr schrieb Jan Engelhardt : > A program may have been built with 4.3 but is combined > with 4.2.3 at runtime While I can only speak for myself, this is not only not supported, it is also something that FFmpeg will not support in the future. Carl Eugen , then this can happen: > > a = avcodec_dct_alloc(); // allocates 896 > #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED > a->get_pixels_unaligned = ffunc; // boom accessing byte ~952 > #endif > > If the API should not be used this way, it should not offer this way. > ___ > 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] ABI break in 4.3
Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt : > This crash is due to Chromium using av_max_alloc in an undocumented way; > more exactly, the Chromium FFmpeg fork changes the allocation functions > so that if max_alloc_size is zero, the limit is completely disabled; and > of course it also sets the limit to zero. Up until 4.2, this worked with > a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead > max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 > this is no longer so. I think it is not immediately obvious that this is a (severe!) issue in Chromium which basically disabled a security feature of FFmpeg that was intentionally set to a very conservative (read: not soo secure) value but was completely disabled by somebody who misunderstood the feature (and failed to ask, I mention this because this person's understanding would have implied that we have no clue in C programming whatsoever). At least one of the "downstream" fixes I saw in the last weeks simply repeat this failure by again removing the security feature instead of removing the wrong call from Chromium. I am not sure if it really is our responsibility to explain to downstream that valid multimedia files theoretically can allocate arbitrary amounts of memory but that a responsible caller has to limit this amount for nearly every theoretical use case, the more so for browser decoding. Carl Eugen ___ 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] How to resolve "ERROR: sdl2 requested but not found" when cross compile?
Am So., 5. Juli 2020 um 12:46 Uhr schrieb JACKY_ZZ[猫猫爱吃鱼] : > > but I got "ERROR: sdl2 requested but not found" message Was this the only message printed on the console? Carl Eugen ___ 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] ABI break in 4.3
On 05.07.2020 14:10, Jan Engelhardt wrote:
On Sunday 2020-07-05 13:39, Tomas Härdin wrote:
Downgrading to a .so file with a lower minor version number than
the program is built against can never be expected to work. Else
we couldn't add new functions without a major bump.
It requires the use ELF symbol versions -- which ffmpeg fails to
do properly.[...]
This is a fair point. I didn't actually know the loader can do stuff
like this, sounds super handy. How hard would it be to get that going?
Change libavcodec.v to
LIBAVCODEC_58 {
global:
av_foo;
av_bar;
av_whatever_else_there_is;...
local:
*;
};
LIBAVCODEC_58.91 {
global:
avpriv_mpeg4audio_get_config2;
} LIBAVCODEC_58;
LIBAVCODEC_58.123 { /* future example */
global:
avblahblah;
} LIBAVCODEC_58.91;
Repeat likewise for other .v files. The file is no longer a template. The
automatic substitution of "_MAJOR" by the build system needs to cease. Version
numbers in the file are supposed to be fixed (among the set of all .so files
that share a SONAME).
Won't that break the entire ABI of anything currently linked, and thus
would require a major bump?
Generally, this seems incredibly hard to maintain and will very likely
cause confusion every time someone adds stuff in the future.
___
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] ABI break in 4.3
On 05.07.2020 16:18, Carl Eugen Hoyos wrote: Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt : This crash is due to Chromium using av_max_alloc in an undocumented way; more exactly, the Chromium FFmpeg fork changes the allocation functions so that if max_alloc_size is zero, the limit is completely disabled; and of course it also sets the limit to zero. Up until 4.2, this worked with a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 this is no longer so. I think it is not immediately obvious that this is a (severe!) issue in Chromium which basically disabled a security feature of FFmpeg that was intentionally set to a very conservative (read: not soo secure) value but was completely disabled by somebody who misunderstood the feature (and failed to ask, I mention this because this person's understanding would have implied that we have no clue in C programming whatsoever). At least one of the "downstream" fixes I saw in the last weeks simply repeat this failure by again removing the security feature instead of removing the wrong call from Chromium. I am not sure if it really is our responsibility to explain to downstream that valid multimedia files theoretically can allocate arbitrary amounts of memory but that a responsible caller has to limit this amount for nearly every theoretical use case, the more so for browser decoding. Carl Eugen Chrome is using a custom allocator, that crashes the entire application on OOM rather than returning NULL. So it's not a security issue in their case. ___ 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] ABI break in 4.3
On Sun, 5 Jul 2020, Timo Rothenpieler wrote: On 05.07.2020 16:18, Carl Eugen Hoyos wrote: Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt : This crash is due to Chromium using av_max_alloc in an undocumented way; more exactly, the Chromium FFmpeg fork changes the allocation functions so that if max_alloc_size is zero, the limit is completely disabled; and of course it also sets the limit to zero. Up until 4.2, this worked with a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 this is no longer so. I think it is not immediately obvious that this is a (severe!) issue in Chromium which basically disabled a security feature of FFmpeg that was intentionally set to a very conservative (read: not soo secure) value but was completely disabled by somebody who misunderstood the feature (and failed to ask, I mention this because this person's understanding would have implied that we have no clue in C programming whatsoever). At least one of the "downstream" fixes I saw in the last weeks simply repeat this failure by again removing the security feature instead of removing the wrong call from Chromium. I am not sure if it really is our responsibility to explain to downstream that valid multimedia files theoretically can allocate arbitrary amounts of memory but that a responsible caller has to limit this amount for nearly every theoretical use case, the more so for browser decoding. Carl Eugen Chrome is using a custom allocator, that crashes the entire application on OOM rather than returning NULL. So it's not a security issue in their case. This is based on the assumption that a libav* function only returns NULL if there is a memory allocation error. That is simply not true. I am sure we can find a function in the codebase which returns NULL because of invalid arguments, or some other condition. IMHO they should not make this assumption. Regards, 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] ABI break in 4.3
sön 2020-07-05 klockan 14:10 +0200 skrev Jan Engelhardt:
> On Sunday 2020-07-05 13:39, Tomas Härdin wrote:
> > > > Downgrading to a .so file with a lower minor version number than
> > > > the program is built against can never be expected to work. Else
> > > > we couldn't add new functions without a major bump.
> > >
> > > It requires the use ELF symbol versions -- which ffmpeg fails to
> > > do properly.[...]
> >
> > This is a fair point. I didn't actually know the loader can do stuff
> > like this, sounds super handy. How hard would it be to get that going?
>
> Change libavcodec.v to
>
> LIBAVCODEC_58 {
> global:
> av_foo;
> av_bar;
> av_whatever_else_there_is;...
> local:
> *;
> };
> LIBAVCODEC_58.91 {
> global:
> avpriv_mpeg4audio_get_config2;
> } LIBAVCODEC_58;
> LIBAVCODEC_58.123 { /* future example */
> global:
> avblahblah;
> } LIBAVCODEC_58.91;
>
> Repeat likewise for other .v files. The file is no longer a template. The
> automatic substitution of "_MAJOR" by the build system needs to cease. Version
> numbers in the file are supposed to be fixed (among the set of all .so files
> that share a SONAME).
Interesting. This also makes what's changed between versions more
explicit. Can this be checked by machine as well? Like having a post-
receive hook that makes sure the .v files are consistent, or maybe have
FATE check it somehow.
This probably needs to be passed through the technical committee. But I
don't think it'll break the ABI like Timo suggests, if we bump minor at
the same time.
/Tomas
___
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] Project orientation
On Sun, 5 Jul 2020, Kieran Kunhya wrote: Would you want something experimental like x262 to be part of the > FFmpeg codebase, for us to have to maintain forever? If you don't limit > scope then that is what would happen. x262 is another example of a fork, where the fork alone was not popular/interesting enough to live on. If it were merged to x264, I am fairly certain it would not be experimental anymore, and we'd have an MPEG2 encoder which would scale much better to multiple cores than what we have now in ffmpeg. Highly unlikely, x264 development has essentially ground to a halt. And people use H.264 a lot still. x262 works well enough for an old format like MPEG-2. There's no real need to develop it unless someone needs to eke out an extra 2% compression because they have a million MPEG-2 receivers they can't change. x264 is practically feature complete, but x262 still miss some things, like 4:2:2 interlaced. Sure, x262 can work well enough for some use cases, but it is still not packaged in e.g. Ubuntu, so users are stuck with the - in some ways - inferior mpeg2 encoder of ffmpeg. The point I am trying to make is that you and some other people made a fast and modern mpeg2 encoder, in some ways superior to existing open source alternatives, but very few people is using it because it was not merged to a bigger/more popular project like x264 or ffmpeg. So it receives no mainteance, no distribution support, no user base and ultimately no further development. Or at least that is how I see it. Regards, Marton The original x264 developers didn't want a merge back by the way. And I'd also like to point to the linux kernel as an example of a monolitic code repository which seems to work quite well. Exception that proves the rule. A lot of developers there are paid full time to work on the kernel and just do cleanup, merge old patches etc. Kieran ___ 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] Project orientation
sön 2020-07-05 klockan 14:28 +0200 skrev Marton Balint: > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: > > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > > > > > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > > > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > > > > wrote: > > > > > > [...] > > > > > > > At some point, the project needs to decide what is in and what is > > > > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > > > > externally. It's not a problem to say "No" to a feature, > > > > > > > especially when, the number of people using that feature is under > > > > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > > > > anyway. > > > > > > > > > > > > what we need is not to say "no" but to say > > > > > > "use this API, implement the module in your own repository, and > > > > > > register your module there" > > > > > > > > > > Once again, Michael, we agree, on this topic. > > > > > > > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > > > > useful". > > > > > > > > I'd like to express a general agreement with this point. The project > > > > has experienced quite a lot of scope creep lately. We don't have to > > > > accommodate everyone, especially with the finite number of developers > > > > available. > > > > > > > > We can encourage people to maintain their own forks of FFmpeg, see for > > > > example FFmbc. > > > > > > And see how dead that project is. Or ffserver. Or x262. > > > > That may be because Baptiste is focusing on other things. The code is > > still there. It still compiles. > > That is the point. Baptise is focusing on other things, and the code he > used to do rots in an external repository. I guess most of the features he > implemented crept back to ffmpeg (I did the backporting of some features > myself), but still, it was duplicate work. It's not duplicate work if the work wouldn't have happened at all in the first place. We could probably work toward merging more features from FFmbc into FFmpeg since presumably Baptiste doesn't depend on the license trolling aspect of the former any more. I could probably do it if Baptiste would be willing to dual license them and we could find a bit of funding for it. > Forks are good, if you are submitting the code back to master. If not, > then forks are not so good because they often dont't live on alone and you > lose the features in it. Here is where scope/feature creep becomes a problem. Features aren't free. Every feature is a liability and needs to be maintained. I for one don't have infinite time to spend. This is why I focus mostly on MXF. > > Would you want something experimental like x262 to be part of the > > FFmpeg codebase, for us to have to maintain forever? If you don't limit > > scope then that is what would happen. > > x262 is another example of a fork, where the fork alone was not > popular/interesting enough to live on. If it were merged to x264, I am > fairly certain it would not be experimental anymore, and we'd have an > MPEG2 encoder which would scale much better to multiple cores than what we > have now in ffmpeg. > > So having something merged and maintained in ffmpeg has the benefit that > more people have the ability to test the code and to develop it. Also it > is more likely for the project to get developers if their code live in the > core ffmpeg respository. I also don't see that maitenance burden to be so > huge. And usefulness is also questionable. I think it is safe to say that > having AMQP/ZMQ protocol support is much more useful then Lego Racers > demuxer... (and I quite liked Lego Racers!!!). > > So my opinion would be to be inclusive when merging stuff. I am not saying > everything has to be merged, I rejected not very long ago the NIT table > parsing or the EPG "data decoder", these were really out of scope and more > importantly out of the current capabilites of the framework. And if > distros are worried about dependencies, they can always make two packages, > a normal and a full. Decent enough points. I just prefer being a bit more conservative with what gets in, due to the maintainance burden. > And I'd also like to point to the linux kernel as an example of a > monolitic code repository which seems to work quite well. Kernel development being quite well-delegated may contribute to this. Perhaps FFmpeg should move more in such a direction? Might be interesting. /Tomas ___ 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 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
On 05-07-2020 06:35 pm, Manolis Stamatogiannakis wrote:
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index c962ac55b0..c4ca39cb6d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,15 +17930,12 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
-@item shift
-Shift subtitles timings by the specified amount.
-
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17952,12 +17950,18 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@item shift
+Shift subtitles timings by the specified amount.
+@end table
+
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
Break this off into a standalone without the shift option entry.
Then merge the doc shift entry with the code patches.
Regards,
Gyan
___
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] ABI break in 4.3
Am So., 5. Juli 2020 um 16:46 Uhr schrieb Timo Rothenpieler : > > On 05.07.2020 16:18, Carl Eugen Hoyos wrote: > > Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt > > : > > > >> This crash is due to Chromium using av_max_alloc in an undocumented way; > >> more exactly, the Chromium FFmpeg fork changes the allocation functions > >> so that if max_alloc_size is zero, the limit is completely disabled; and > >> of course it also sets the limit to zero. Up until 4.2, this worked with > >> a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead > >> max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 > >> this is no longer so. > > > > I think it is not immediately obvious that this is a (severe!) issue in > > Chromium which basically disabled a security feature of FFmpeg > > that was intentionally set to a very conservative (read: not soo > > secure) value but was completely disabled by somebody who > > misunderstood the feature (and failed to ask, I mention this > > because this person's understanding would have implied that we > > have no clue in C programming whatsoever). > > > > At least one of the "downstream" fixes I saw in the last weeks simply > > repeat this failure by again removing the security feature instead of > > removing the wrong call from Chromium. > > > > I am not sure if it really is our responsibility to explain to downstream > > that valid multimedia files theoretically can allocate arbitrary amounts > > of memory but that a responsible caller has to limit this amount for > > nearly every theoretical use case, the more so for browser decoding. > > > > Carl Eugen > > Chrome is using a custom allocator, that crashes the entire application > on OOM rather than returning NULL. > So it's not a security issue in their case. I understood the situation differently (because this is not about oom): FFmpeg (by default) limits the possible allocation of a single call to malloc() - independently of the used memory allocator. The default limit is extremely high and it is difficult to image a useful file that reaches the limit. Somebody at Google thought that the "limitation" means that libavutil returns only a block of "limit" if a block bigger than limit was requested and therefore decided to remove the limit. Apart from the fact that this would be a very severe issue in FFmpeg that this Google employee decided not to report to us, in addition the removal of the limit means that it is possible to allocate nearly the complete memory from a media file within the browser which clearly is a security issue in my opinion. (I have no idea if their allocator limits the max possible allocation anyway but even if it does, there should be no reason for their change.) Carl Eugen ___ 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] ABI break in 4.3
Am So., 5. Juli 2020 um 16:44 Uhr schrieb Timo Rothenpieler : > Generally, this seems incredibly hard to maintain and will very likely > cause confusion every time someone adds stuff in the future. True, and this is while this will not even reach the committee. Carl Eugen ___ 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] ABI break in 4.3
On Sunday 2020-07-05 16:43, Timo Rothenpieler wrote:
>> LIBAVCODEC_58 {
>>global:
>>av_foo;
>>av_bar;
>>av_whatever_else_there_is;...
>>local:
>>*;
>> };
>> LIBAVCODEC_58.91 {
>>global:
>>avpriv_mpeg4audio_get_config2;
>> } LIBAVCODEC_58;
>> LIBAVCODEC_58.123 { /* future example */
>>global:
>>avblahblah;
>> } LIBAVCODEC_58.91;
>
> Won't that break the entire ABI of anything currently linked, and thus would
> require a major bump?
Since 4.3 was sort of a break over 4.2.3 already, the situation is that:
* $nextrelease can be compatible with 4.2.3's idea of the ABI, in which case
avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.91, or
* $nextrelease can be compatible with 4.3's idea of the ABI, in which case
avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.
Each of the two prior options is equally non-compelling. "58" is tarnished
already. What software generally does at this point — ffmpeg is not the first
project to have these issues — is to bump and start fresh.
> Generally, this seems incredibly hard to maintain and will very likely cause
> confusion every time someone adds stuff in the future.
How often do exported functions get added? Between 4.2.3 and 4.3,
that was _just one_, and that one was even an avpriv_* (which
probably shouldn't have been exported given its "priv" nature?!).
___
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 1/4] kmsgrab: Refactor and clean error cases
---
libavdevice/kmsgrab.c | 151 ++
1 file changed, 93 insertions(+), 58 deletions(-)
diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index d0de774871..47ba15ca07 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -81,70 +81,42 @@ static void kmsgrab_free_frame(void *opaque, uint8_t *data)
av_frame_free(&frame);
}
-static int kmsgrab_read_packet(AVFormatContext *avctx, AVPacket *pkt)
+static int kmsgrab_get_fb(AVFormatContext *avctx,
+ drmModePlane *plane,
+ AVDRMFrameDescriptor *desc)
{
KMSGrabContext *ctx = avctx->priv_data;
-drmModePlane *plane;
-drmModeFB *fb;
-AVDRMFrameDescriptor *desc;
-AVFrame *frame;
-int64_t now;
+drmModeFB *fb = NULL;
int err, fd;
-now = av_gettime();
-if (ctx->frame_last) {
-int64_t delay;
-while (1) {
-delay = ctx->frame_last + ctx->frame_delay - now;
-if (delay <= 0)
-break;
-av_usleep(delay);
-now = av_gettime();
-}
-}
-ctx->frame_last = now;
-
-plane = drmModeGetPlane(ctx->hwctx->fd, ctx->plane_id);
-if (!plane) {
-av_log(avctx, AV_LOG_ERROR, "Failed to get plane "
- "%"PRIu32".\n", ctx->plane_id);
-return AVERROR(EIO);
-}
-if (!plane->fb_id) {
-av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" no longer has "
- "an associated framebuffer.\n", ctx->plane_id);
-return AVERROR(EIO);
-}
-
fb = drmModeGetFB(ctx->hwctx->fd, plane->fb_id);
if (!fb) {
av_log(avctx, AV_LOG_ERROR, "Failed to get framebuffer "
"%"PRIu32".\n", plane->fb_id);
-return AVERROR(EIO);
+err = AVERROR(EIO);
+goto fail;
}
if (fb->width != ctx->width || fb->height != ctx->height) {
av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer "
"dimensions changed: now %"PRIu32"x%"PRIu32".\n",
ctx->plane_id, fb->width, fb->height);
-return AVERROR(EIO);
+err = AVERROR(EIO);
+goto fail;
}
if (!fb->handle) {
av_log(avctx, AV_LOG_ERROR, "No handle set on framebuffer.\n");
-return AVERROR(EIO);
+err = AVERROR(EIO);
+goto fail;
}
err = drmPrimeHandleToFD(ctx->hwctx->fd, fb->handle, O_RDONLY, &fd);
if (err < 0) {
-err = errno;
+err = AVERROR(errno);
av_log(avctx, AV_LOG_ERROR, "Failed to get PRIME fd from "
"framebuffer handle: %s.\n", strerror(errno));
-return AVERROR(err);
+goto fail;
}
-desc = av_mallocz(sizeof(*desc));
-if (!desc)
-return AVERROR(ENOMEM);
-
*desc = (AVDRMFrameDescriptor) {
.nb_objects = 1,
.objects[0] = {
@@ -164,31 +136,92 @@ static int kmsgrab_read_packet(AVFormatContext *avctx,
AVPacket *pkt)
},
};
+err = 0;
+fail:
+drmModeFreeFB(fb);
+return err;
+}
+
+static int kmsgrab_read_packet(AVFormatContext *avctx, AVPacket *pkt)
+{
+KMSGrabContext *ctx = avctx->priv_data;
+drmModePlane *plane = NULL;
+AVDRMFrameDescriptor *desc = NULL;
+AVFrame *frame = NULL;
+int64_t now;
+int err;
+
+now = av_gettime();
+if (ctx->frame_last) {
+int64_t delay;
+while (1) {
+delay = ctx->frame_last + ctx->frame_delay - now;
+if (delay <= 0)
+break;
+av_usleep(delay);
+now = av_gettime();
+}
+}
+ctx->frame_last = now;
+
+plane = drmModeGetPlane(ctx->hwctx->fd, ctx->plane_id);
+if (!plane) {
+av_log(avctx, AV_LOG_ERROR, "Failed to get plane "
+ "%"PRIu32".\n", ctx->plane_id);
+err = AVERROR(EIO);
+goto fail;
+}
+if (!plane->fb_id) {
+av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" no longer has "
+ "an associated framebuffer.\n", ctx->plane_id);
+err = AVERROR(EIO);
+goto fail;
+}
+
+desc = av_mallocz(sizeof(*desc));
+if (!desc) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
+
+err = kmsgrab_get_fb(avctx, plane, desc);
+if (err < 0)
+goto fail;
+
frame = av_frame_alloc();
-if (!frame)
-return AVERROR(ENOMEM);
+if (!frame) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
frame->hw_frames_ctx = av_buffer_ref(ctx->frames_ref);
-if (!frame->hw_frames_ctx)
-return AVERROR(ENOMEM);
+if (!frame->hw_frames_ctx) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
frame->buf[0] = av_buffer_create((uint8_t*)desc, sizeof(*desc),
&kmsgrab_free_desc, avctx, 0);
-if (!frame->buf[0])
-return AVERROR(ENOMEM);
+if (!frame->buf[0]) {
+err = AVERROR(ENOMEM);
+goto fail;
+}
frame->dat
[FFmpeg-devel] [PATCH 4/4] doc/indevs: Note improved behaviour of kmsgrab with Linux 5.7
---
doc/indevs.texi | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/doc/indevs.texi b/doc/indevs.texi
index 0f33fc66d8..4d2312e201 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -889,11 +889,15 @@ If you don't understand what all of that means, you
probably don't want this. L
DRM device to capture on. Defaults to @option{/dev/dri/card0}.
@item format
-Pixel format of the framebuffer. Defaults to @option{bgr0}.
+Pixel format of the framebuffer. This can be autodetected if you are running
Linux 5.7
+or later, but needs to be provided for earlier versions. Defaults to
@option{bgr0},
+which is the most common format used by the Linux console and Xorg X server.
@item format_modifier
Format modifier to signal on output frames. This is necessary to import
correctly into
-some APIs, but can't be autodetected. See the libdrm documentation for
possible values.
+some APIs. It can be autodetected if you are running Linux 5.7 or later, but
will need
+to be provided explicitly when needed in earlier versions. See the libdrm
documentation
+for possible values.
@item crtc_id
KMS CRTC ID to define the capture source. The first active plane on the given
CRTC
--
2.27.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] lavfi/vf_vpp_qsv: fix the infinite loop while framerate lower than input
> > I can't see the benefit to use MSDK framerate conversion function. Is > > it a good idea to drop it and use ffmpeg native fps filter instead? > > Reconsidering this, leaving the filter buggy doesn't seem to be comfortable > to me, > hence IMHO it'll be better to have this bug fixed. My suggestion would be just delete MSDK framerate conversion instead of patch it. Will send a patch if nobody against. ___ 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 3/4] kmsgrab: Don't require the user to set framebuffer format
This is provided by GetFB2, but we still need the option for cases where
that isn't available.
---
libavdevice/kmsgrab.c | 55 +++
1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index 3e89c3f445..b859f202aa 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -405,18 +405,6 @@ static av_cold int kmsgrab_read_header(AVFormatContext
*avctx)
AVStream *stream;
int err, i;
-for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) {
-if (kmsgrab_formats[i].pixfmt == ctx->format) {
-ctx->drm_format = kmsgrab_formats[i].drm_format;
-break;
-}
-}
-if (i >= FF_ARRAY_ELEMS(kmsgrab_formats)) {
-av_log(avctx, AV_LOG_ERROR, "Unsupported format %s.\n",
- av_get_pix_fmt_name(ctx->format));
-return AVERROR(EINVAL);
-}
-
err = av_hwdevice_ctx_create(&ctx->device_ref, AV_HWDEVICE_TYPE_DRM,
ctx->device_path, NULL, 0);
if (err < 0) {
@@ -530,9 +518,25 @@ static av_cold int kmsgrab_read_header(AVFormatContext
*avctx)
err = AVERROR(EINVAL);
goto fail;
}
-if (ctx->drm_format != fb2->pixel_format) {
+
+for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) {
+if (kmsgrab_formats[i].drm_format == fb2->pixel_format) {
+if (ctx->format != AV_PIX_FMT_NONE &&
+ctx->format != kmsgrab_formats[i].pixfmt) {
+av_log(avctx, AV_LOG_ERROR, "Framebuffer pixel format "
+ "%"PRIx32" does not match expected format.\n",
+ fb2->pixel_format);
+err = AVERROR(EINVAL);
+goto fail;
+}
+ctx->drm_format = fb2->pixel_format;
+ctx->format = kmsgrab_formats[i].pixfmt;
+break;
+}
+}
+if (i == FF_ARRAY_ELEMS(kmsgrab_formats)) {
av_log(avctx, AV_LOG_ERROR, "Framebuffer pixel format "
- "%"PRIx32" does not match expected format.\n",
+ "%"PRIx32" is not a known supported format.\n",
fb2->pixel_format);
err = AVERROR(EINVAL);
goto fail;
@@ -547,11 +551,32 @@ static av_cold int kmsgrab_read_header(AVFormatContext
*avctx)
} else {
ctx->drm_format_modifier = fb2->modifier;
}
+av_log(avctx, AV_LOG_VERBOSE, "Format is %s, from "
+ "DRM format %"PRIx32" modifier %"PRIx64".\n",
+ av_get_pix_fmt_name(ctx->format),
+ ctx->drm_format, ctx->drm_format_modifier);
+
ctx->fb2_available = 1;
}
#endif
if (!ctx->fb2_available) {
+if (ctx->format == AV_PIX_FMT_NONE) {
+// Backward compatibility: assume BGR0 if no format supplied.
+ctx->format = AV_PIX_FMT_BGR0;
+}
+for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) {
+if (kmsgrab_formats[i].pixfmt == ctx->format) {
+ctx->drm_format = kmsgrab_formats[i].drm_format;
+break;
+}
+}
+if (i >= FF_ARRAY_ELEMS(kmsgrab_formats)) {
+av_log(avctx, AV_LOG_ERROR, "Unsupported format %s.\n",
+ av_get_pix_fmt_name(ctx->format));
+return AVERROR(EINVAL);
+}
+
fb = drmModeGetFB(ctx->hwctx->fd, plane->fb_id);
if (!fb) {
err = errno;
@@ -642,7 +667,7 @@ static const AVOption options[] = {
{ .str = "/dev/dri/card0" }, 0, 0, FLAGS },
{ "format", "Pixel format for framebuffer",
OFFSET(format), AV_OPT_TYPE_PIXEL_FMT,
- { .i64 = AV_PIX_FMT_BGR0 }, 0, UINT32_MAX, FLAGS },
+ { .i64 = AV_PIX_FMT_NONE }, -1, INT32_MAX, FLAGS },
{ "format_modifier", "DRM format modifier for framebuffer",
OFFSET(drm_format_modifier), AV_OPT_TYPE_INT64,
{ .i64 = DRM_FORMAT_MOD_INVALID }, 0, INT64_MAX, FLAGS },
--
2.27.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".
[FFmpeg-devel] [PATCH 2/4] kmsgrab: Use GetFB2 if available
The most useful feature here is the ability to automatically extract the
framebuffer format and modifiers. It also makes support for multi-plane
framebuffers possible, though they will need to be added to the format
table to work (not tested by me).
This requires libdrm 2.4.101 (from April 2020) to build, so it includes a
configure check to allow compatibility with existing distributions. Even
with libdrm support, it still won't do anything at runtime if you are
running Linux < 5.7 (before June 2020).
---
This has been hanging around for a while waiting for the GETFB2 ioctl() to
actually make it into stable Linux, which it now is with 5.7.
configure | 4 +
libavdevice/kmsgrab.c | 221 +-
2 files changed, 203 insertions(+), 22 deletions(-)
diff --git a/configure b/configure
index bdfd731602..2900acc687 100755
--- a/configure
+++ b/configure
@@ -2323,6 +2323,7 @@ HAVE_LIST="
$THREADS_LIST
$TOOLCHAIN_FEATURES
$TYPES_LIST
+libdrm_getfb2
makeinfo
makeinfo_html
opencl_d3d11
@@ -6631,6 +6632,9 @@ test_cpp <= 0.35.0" "va/va.h" vaInitialize
diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index 47ba15ca07..3e89c3f445 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -27,6 +27,11 @@
#include
#include
+// Required for compatibility when building against libdrm < 2.4.83.
+#ifndef DRM_FORMAT_MOD_INVALID
+#define DRM_FORMAT_MOD_INVALID ((1ULL << 56) - 1)
+#endif
+
#include "libavutil/hwcontext.h"
#include "libavutil/hwcontext_drm.h"
#include "libavutil/internal.h"
@@ -45,6 +50,7 @@ typedef struct KMSGrabContext {
AVBufferRef *device_ref;
AVHWDeviceContext *device;
AVDRMDeviceContext *hwctx;
+int fb2_available;
AVBufferRef *frames_ref;
AVHWFramesContext *frames;
@@ -68,8 +74,10 @@ typedef struct KMSGrabContext {
static void kmsgrab_free_desc(void *opaque, uint8_t *data)
{
AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor*)data;
+int i;
-close(desc->objects[0].fd);
+for (i = 0; i < desc->nb_objects; i++)
+close(desc->objects[i].fd);
av_free(desc);
}
@@ -142,6 +150,114 @@ fail:
return err;
}
+#if HAVE_LIBDRM_GETFB2
+static int kmsgrab_get_fb2(AVFormatContext *avctx,
+ drmModePlane *plane,
+ AVDRMFrameDescriptor *desc)
+{
+KMSGrabContext *ctx = avctx->priv_data;
+drmModeFB2 *fb;
+int err, i, nb_objects;
+
+fb = drmModeGetFB2(ctx->hwctx->fd, plane->fb_id);
+if (!fb) {
+av_log(avctx, AV_LOG_ERROR, "Failed to get framebuffer "
+ "%"PRIu32".\n", plane->fb_id);
+return AVERROR(EIO);
+}
+if (fb->pixel_format != ctx->drm_format) {
+av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer "
+ "format changed: now %"PRIx32".\n",
+ ctx->plane_id, fb->pixel_format);
+err = AVERROR(EIO);
+goto fail;
+}
+if (fb->modifier != ctx->drm_format_modifier) {
+av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer "
+ "format modifier changed: now %"PRIx64".\n",
+ ctx->plane_id, fb->modifier);
+err = AVERROR(EIO);
+goto fail;
+}
+if (fb->width != ctx->width || fb->height != ctx->height) {
+av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer "
+ "dimensions changed: now %"PRIu32"x%"PRIu32".\n",
+ ctx->plane_id, fb->width, fb->height);
+err = AVERROR(EIO);
+goto fail;
+}
+if (!fb->handles[0]) {
+av_log(avctx, AV_LOG_ERROR, "No handle set on framebuffer.\n");
+err = AVERROR(EIO);
+goto fail;
+}
+
+*desc = (AVDRMFrameDescriptor) {
+.nb_layers = 1,
+.layers[0] = {
+.format = ctx->drm_format,
+},
+};
+
+nb_objects = 0;
+for (i = 0; i < 4 && fb->handles[i]; i++) {
+size_t size;
+int dup = 0, j, obj;
+
+size = fb->offsets[i] + fb->height * fb->pitches[i];
+
+for (j = 0; j < i; j++) {
+if (fb->handles[i] == fb->handles[j]) {
+dup = 1;
+break;
+}
+}
+if (dup) {
+obj = desc->layers[0].planes[j].object_index;
+
+if (desc->objects[j].size < size)
+desc->objects[j].size = size;
+
+desc->layers[0].planes[i] = (AVDRMPlaneDescriptor) {
+.object_index = obj,
+.offset = fb->offsets[i],
+.pitch= fb->pitches[i],
+};
+
+} else {
+int fd;
+err = drmPrimeHandleToFD(ctx->hwctx->fd, fb->handles[i],
+ O_RDONLY, &fd);
+if (err < 0) {
+err = AVERROR(errno);
+av_log(avctx, AV_LOG_ERROR, "Failed to get PRIME fd from "
+ "framebuffer handle: %
Re: [FFmpeg-devel] patch submission questions
Thanks for the responses Hongcheng and Steinar! I'll use '-n' with git format-patch if I need to resubmit. Patches already work as Steinar describes. In the meantime patch 2/3 found its way to patchwork. The delay was probably due to some email processing hiccup. So all is good! Cheers, Manolis On Sun, 5 Jul 2020 at 16:06, Steinar H. Gunderson < steinar+ffm...@gunderson.no> wrote: > On Sun, Jul 05, 2020 at 03:42:34PM +0200, Manolis Stamatogiannakis wrote: > > Q2: In a patchset consisting of several commits, is each commit expected > to > > be "standalone"? I.e. does it have to apply cleanly without depending on > > the previous commits in the patchset? > > No, but it has to compile and work even if not applying any of the latter > patches. > (This increases reviewability and bisectability.) AFAIK Patchwork doesn't > enforce this, though. > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ > ___ > 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] ABI break in 4.3
On 05.07.2020 17:46, Jan Engelhardt wrote:
On Sunday 2020-07-05 16:43, Timo Rothenpieler wrote:
LIBAVCODEC_58 {
global:
av_foo;
av_bar;
av_whatever_else_there_is;...
local:
*;
};
LIBAVCODEC_58.91 {
global:
avpriv_mpeg4audio_get_config2;
} LIBAVCODEC_58;
LIBAVCODEC_58.123 { /* future example */
global:
avblahblah;
} LIBAVCODEC_58.91;
Won't that break the entire ABI of anything currently linked, and thus would
require a major bump?
Since 4.3 was sort of a break over 4.2.3 already, the situation is that:
It wasn't. Structs expanding at the end have occurred multiple times in
the past already, and are documented as not having their size be part of
the ABI/API.
Also, no functions were removed.
* $nextrelease can be compatible with 4.2.3's idea of the ABI, in which case
avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.91, or
* $nextrelease can be compatible with 4.3's idea of the ABI, in which case
avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.
Each of the two prior options is equally non-compelling. "58" is tarnished
already. What software generally does at this point — ffmpeg is not the first
project to have these issues — is to bump and start fresh.
Generally, this seems incredibly hard to maintain and will very likely cause
confusion every time someone adds stuff in the future.
How often do exported functions get added? Between 4.2.3 and 4.3,
that was _just one_, and that one was even an avpriv_* (which
probably shouldn't have been exported given its "priv" nature?!).
Sometimes the libraries have functions they access from each other that
are not meant to be public, but still need to be exported.
___
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] ABI break in 4.3
On 7/5/2020 12:46 PM, Jan Engelhardt wrote:
>
> On Sunday 2020-07-05 16:43, Timo Rothenpieler wrote:
>>> LIBAVCODEC_58 {
>>>global:
>>>av_foo;
>>>av_bar;
>>>av_whatever_else_there_is;...
>>>local:
>>>*;
>>> };
>>> LIBAVCODEC_58.91 {
>>>global:
>>>avpriv_mpeg4audio_get_config2;
>>> } LIBAVCODEC_58;
>>> LIBAVCODEC_58.123 { /* future example */
>>>global:
>>>avblahblah;
>>> } LIBAVCODEC_58.91;
>>
>> Won't that break the entire ABI of anything currently linked, and thus would
>> require a major bump?
>
> Since 4.3 was sort of a break over 4.2.3 already
No, it wasn't. No field offsets were changed, no public structs where
their size is part of the ABI were altered, and no public symbols were
removed. The situation is exactly the same as when 4.2 was released
after 4.1, and every other release before those.
If you can run 4.3 at runtime on a program that linked to 4.2, then it's
working as intended. Attempting the inverse is not and will never be
allowed or considered a valid scenario.
And i want to stress the fact that the reported Chromium breakage and
most assuredly also the Blender breakage are unrelated to what you're
arguing about in this thread.
> , the situation is that:
>
> * $nextrelease can be compatible with 4.2.3's idea of the ABI, in which case
>avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.91, or
>
> * $nextrelease can be compatible with 4.3's idea of the ABI, in which case
>avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.
>
> Each of the two prior options is equally non-compelling. "58" is tarnished
> already.
As "tarnished" as every previous soname was, if we go by your definition
of breakage. I don't recall it ever being an issue for anyone until now.
This nonetheless is a good proposal and can be considered for the next
soname bump, if it can help prevent people using the wrong release at
runtime.
> What software generally does at this point — ffmpeg is not the first
> project to have these issues — is to bump and start fresh.
>
>
>> Generally, this seems incredibly hard to maintain and will very likely cause
>> confusion every time someone adds stuff in the future.
>
> How often do exported functions get added? Between 4.2.3 and 4.3,
> that was _just one_, and that one was even an avpriv_* (which
> probably shouldn't have been exported given its "priv" nature?!).
avpriv_* are inter library communication functions. An unfortunate
consequence of having the project split across half a dozen libraries.
They need to be exported, but not exposed in the installed headers.
___
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] Project orientation
On Sun, 5 Jul 2020, Kieran Kunhya wrote: x264 is practically feature complete, but x262 still miss some things, like 4:2:2 interlaced. Sure, x262 can work well enough for some use cases, but it is still not packaged in e.g. Ubuntu, so users are stuck with the - in some ways - inferior mpeg2 encoder of ffmpeg. The point I am trying to make is that you and some other people made a fast and modern mpeg2 encoder, in some ways superior to existing open source alternatives, but very few people is using it because it was not merged to a bigger/more popular project like x264 or ffmpeg. So it receives no mainteance, no distribution support, no user base and ultimately no further development. Or at least that is how I see it. People aren't using it because people don't use MPEG-2. There is no maintenance to be done on a format that's 25 years old, do you want me to randomly change cosmetics to make you feel happy? If you'd get off your high horse for once, that would make me feel happy. I know of people using it 24/7 for many years and have had no issues. Have you opened bug reports on x264-devel about the issues you see? MPEG-2 4:2:2 interlaced users are infinitesimally small and professional users. If you want your pet feature to work, lo and behold you have to work on it or pay someone to work on it. If you want people to work on it out of nowhere then you are part of the Free Rider Problem: https://en.wikipedia.org/wiki/Free-rider_problem I don't think I told anybody to implement the missing features for free. But I do believe that payed or free development has a higher chance of happening if existing code is already available in a popular package/project. And yes, I believe that some "maintenance burden" should be accepted by the base project to give more chance to further advancement, payed or free. Regards, 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".
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 11
libavfilter/vf_subtitles.c | 54 +-
2 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index d2b8feb14b..a8af563551 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17936,6 +17936,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17949,6 +17952,9 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+
+@item shift
+Shift subtitles timings by the specified amount.
@end table
Additional options for @ref{subtitles} filter:
@@ -17995,6 +18001,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour=&HCCFF'
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..09cca6aab8 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -66,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -227,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass);
static av_cold int init_ass(AVFilterContext *ctx)
{
+int eid, nskip;
+ASS_Event *event;
AssContext *ass = ctx->priv;
int ret = init(ctx);
@@ -243,6 +252,24 @@ static av_cold int init_ass(AVFilterContext *ctx)
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..d2b8feb14b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,7 +17930,7 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
@@ -17948,13 +17949,16 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@end table
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
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] ABI break in 4.3
On Sunday 2020-07-05 18:18, James Almer wrote: >>> >>> Won't that break the entire ABI of anything currently linked, and thus would >>> require a major bump? >> >> Since 4.3 was sort of a break over 4.2.3 already > >No, it wasn't. Perhaps not on a high level. But for the ELF system, it was a break, because you used the tuple, specifically , with two _different sets of contained symbols_. Was it the reason for blender crash? I do not know that, nor do I know the blender internals, but if it can be ruled out (at the very least, in the future) that version mixup between $buildhost and $userhost can be the cause of present or future crashes in blender or otherwise, I'll be damned if I didn't try. === The following changes since commit c2d000ec27af1a5cd5341a67e941e0313879ab18: lavc/qsvenc_hevc: add qmax/qmin support for HEVC encoding (2020-07-05 23:43:45 +0800) are available in the Git repository at: git://github.com/jengelh/ffmpeg master for you to fetch changes up to 3ec24e4e548ecd6d4cc2f11a7d6717548abdadab: build: do proper ELF symbol versioning (2020-07-05 18:50:58 +0200) Jan Engelhardt (1): build: do proper ELF symbol versioning libavcodec/libavcodec.v | 254 +++- libavdevice/libavdevice.v | 28 +- libavfilter/libavfilter.v | 78 - libavformat/libavformat.v | 185 +++- libavresample/libavresample.v | 30 +- libavutil/libavutil.v | 555 +- libswresample/libswresample.v | 33 +- libswscale/libswscale.v | 44 ++- 8 files changed, 1163 insertions(+), 44 deletions(-) ___ 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 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
Fair point. I've submitted a v2 where the docs reorganization is applied
first.
On Sun, 5 Jul 2020 at 17:30, Gyan Doshi wrote:
>
>
> On 05-07-2020 06:35 pm, Manolis Stamatogiannakis wrote:
> > Some options are common between subtitles/ass filters.
> > Rather than mentioning for each option whether it is common or not,
> > the options are now displayed in two separate tables.
> >
> > Signed-off-by: Manolis Stamatogiannakis
> > ---
> > doc/filters.texi | 18 +++---
> > 1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index c962ac55b0..c4ca39cb6d 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands}
> that corresponds to option of
> > @item planes
> > @end table
> >
> > +@anchor{ass}
> > @section ass
> >
> > Same as the @ref{subtitles} filter, except that it doesn't require
> libavcodec
> > @@ -17929,15 +17930,12 @@ To enable compilation of this filter you need
> to configure FFmpeg with
> > libavformat to convert the passed subtitles file to ASS (Advanced
> Substation
> > Alpha) subtitles format.
> >
> > -The filter accepts the following options:
> > +Common @ref{subtitles}/@ref{ass} filter options:
> >
> > @table @option
> > @item filename, f
> > Set the filename of the subtitle file to read. It must be specified.
> >
> > -@item shift
> > -Shift subtitles timings by the specified amount.
> > -
> > @item original_size
> > Specify the size of the original video, the video for which the ASS
> file
> > was composed. For the syntax of this option, check the
> > @@ -17952,12 +17950,18 @@ These fonts will be used in addition to
> whatever the font provider uses.
> > @item alpha
> > Process alpha channel, by default alpha channel is untouched.
> >
> > +@item shift
> > +Shift subtitles timings by the specified amount.
> > +@end table
> > +
> > +Additional options for @ref{subtitles} filter:
> > +
> > +@table @option
> > @item charenc
> > -Set subtitles input character encoding. @code{subtitles} filter only.
> Only
> > -useful if not UTF-8.
> > +Set subtitles input character encoding. Only useful if not UTF-8.
> >
> > @item stream_index, si
> > -Set subtitles stream index. @code{subtitles} filter only.
> > +Set subtitles stream index.
>
> Break this off into a standalone without the shift option entry.
> Then merge the doc shift entry with the code patches.
>
> Regards,
> Gyan
> ___
> 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] ABI break in 4.3
On 7/5/2020 1:58 PM, Jan Engelhardt wrote: > > On Sunday 2020-07-05 18:18, James Almer wrote: Won't that break the entire ABI of anything currently linked, and thus would require a major bump? >>> >>> Since 4.3 was sort of a break over 4.2.3 already >> >> No, it wasn't. > > Perhaps not on a high level. But for the ELF system, it was a break, > because you used the tuple, specifically LIBAVCODEC_58>, with two _different sets of contained symbols_. > > Was it the reason for blender crash? I do not know that, nor do I know the > blender internals, but if it can be ruled out (at the very least, in the > future) that version mixup between $buildhost and $userhost can be the > cause of present or future crashes in blender or otherwise, I'll be damned if > I > didn't try. > > === > > The following changes since commit c2d000ec27af1a5cd5341a67e941e0313879ab18: > > lavc/qsvenc_hevc: add qmax/qmin support for HEVC encoding (2020-07-05 > 23:43:45 +0800) > > are available in the Git repository at: > > git://github.com/jengelh/ffmpeg master > > for you to fetch changes up to 3ec24e4e548ecd6d4cc2f11a7d6717548abdadab: > > build: do proper ELF symbol versioning (2020-07-05 18:50:58 +0200) > > > Jan Engelhardt (1): > build: do proper ELF symbol versioning > > libavcodec/libavcodec.v | 254 +++- > libavdevice/libavdevice.v | 28 +- > libavfilter/libavfilter.v | 78 - > libavformat/libavformat.v | 185 +++- > libavresample/libavresample.v | 30 +- > libavutil/libavutil.v | 555 +- > libswresample/libswresample.v | 33 +- > libswscale/libswscale.v | 44 ++- > 8 files changed, 1163 insertions(+), 44 deletions(-) The list of functions is incomplete. After a quick look, you're for example missing av_map_videotoolbox_format_* in libavutil. This is another issue that i don't know if you considered, and that's the fact currently some symbols may not present depending on configure time options, build environment, and target system. Videotoolbox is of course not available for you unless you're targeting OSX. This afaik can be solved by ensuring they are always present, and returning ENOSYS/NULL as required if the actual implementation is not compiled in. We're doing as much in a few places, but clearly not enough care was taken to ensure that's always the case. ___ 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] Project orientation
On Sun, 5 Jul 2020 at 19:01, Kieran Kunhya wrote: > > For new contributors git send-email is annoying. For people wanting to > push, the .mbox format is annoying, Gmail doesn't support it any more. > And you can't get new contributors to start using CLI based email > clients or run their own mail server, that's not going to happen. > As a fresh contributor, setting up git send-email was a hassle, but not an insurmountable obstacle. Though I'd argue that git send-email is a bigger liability for regular developers. A lot of developers seem to be using Gmail, which means they need to have "less secure apps" enabled at the time they use git send-email. So they are forced to choose between security and convenience. I.e. having to temporarily enable "less secure apps" every time they submit, or accept the risk of weakened security for their account. > > A solution like Gitlab is the only way forward. It has worked well for > dav1d, it can run regression tests on all platforms for all commits: > https://code.videolan.org/videolan/dav1d > > Merges are done with one push of a button. Yes, the branch sprawl is > not great but it's better than now. > It has inline patch reviews which are nice. > FWIW, I'd add that branch-based PRs as implemented with GitHub greatly reduce the turnaround for receiving feedback and addressing the raised issues. All you have to do is force-push on your PR branch, and the PR is cleanly updated. This is in contrast with having to use git send-email, where the emails containing the stale patches will continue to litter patchwork and your mailboxes. Also, IIRC, when you update a branch, the regression tests for all pending PRs on the branch are automatically rerun. This should help address the conflicts in the pending PRs sooner rather than later. Not sure what the status of GitLab is wrt to these features, but I'd expect it to not be very far behind. Best regards, Manolis ___ 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] ABI break in 4.3
On Sun, Jul 5, 2020 at 7:29 PM James Almer wrote: > > On 7/5/2020 1:58 PM, Jan Engelhardt wrote: > > > > On Sunday 2020-07-05 18:18, James Almer wrote: > > Won't that break the entire ABI of anything currently linked, and thus > would > require a major bump? > >>> > >>> Since 4.3 was sort of a break over 4.2.3 already > >> > >> No, it wasn't. > > > > Perhaps not on a high level. But for the ELF system, it was a break, > > because you used the tuple, specifically > LIBAVCODEC_58>, with two _different sets of contained symbols_. > > > > Was it the reason for blender crash? I do not know that, nor do I know the > > blender internals, but if it can be ruled out (at the very least, in the > > future) that version mixup between $buildhost and $userhost can be the > > cause of present or future crashes in blender or otherwise, I'll be damned > > if I > > didn't try. > > > > === > > > > The following changes since commit c2d000ec27af1a5cd5341a67e941e0313879ab18: > > > > lavc/qsvenc_hevc: add qmax/qmin support for HEVC encoding (2020-07-05 > > 23:43:45 +0800) > > > > are available in the Git repository at: > > > > git://github.com/jengelh/ffmpeg master > > > > for you to fetch changes up to 3ec24e4e548ecd6d4cc2f11a7d6717548abdadab: > > > > build: do proper ELF symbol versioning (2020-07-05 18:50:58 +0200) > > > > > > Jan Engelhardt (1): > > build: do proper ELF symbol versioning > > > > libavcodec/libavcodec.v | 254 +++- > > libavdevice/libavdevice.v | 28 +- > > libavfilter/libavfilter.v | 78 - > > libavformat/libavformat.v | 185 +++- > > libavresample/libavresample.v | 30 +- > > libavutil/libavutil.v | 555 +- > > libswresample/libswresample.v | 33 +- > > libswscale/libswscale.v | 44 ++- > > 8 files changed, 1163 insertions(+), 44 deletions(-) > > The list of functions is incomplete. After a quick look, you're for > example missing av_map_videotoolbox_format_* in libavutil. > > This is another issue that i don't know if you considered, and that's > the fact currently some symbols may not present depending on configure > time options, build environment, and target system. Videotoolbox is of > course not available for you unless you're targeting OSX. > > This afaik can be solved by ensuring they are always present, and > returning ENOSYS/NULL as required if the actual implementation is not > compiled in. > We're doing as much in a few places, but clearly not enough care was > taken to ensure that's always the case. I don't think platform specific functionality should get such a treatment. A build on the same platform should always present the same ABI, but nothing you can do will make videotoolbox function on Linux or Windows, so the symbols have no business being included. Headers for such functionality might even require OS-specific headers to be included (eg. D3D on Windows, VT on OSX), which if of course not going to work. In this case, the ABI is not dependent on configure, but on the underlying platform you are building on. Of course it should be corrected if they are purely configure-controlled, and instead perhaps moved to a platform model. - Hendrik ___ 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 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters.
Rather than mentioning for each option whether it is common or not,
the options are now displayed in two separate tables.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index ad2448acb2..d2b8feb14b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that
corresponds to option of
@item planes
@end table
+@anchor{ass}
@section ass
Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
@@ -17929,7 +17930,7 @@ To enable compilation of this filter you need to
configure FFmpeg with
libavformat to convert the passed subtitles file to ASS (Advanced Substation
Alpha) subtitles format.
-The filter accepts the following options:
+Common @ref{subtitles}/@ref{ass} filter options:
@table @option
@item filename, f
@@ -17948,13 +17949,16 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+@end table
+Additional options for @ref{subtitles} filter:
+
+@table @option
@item charenc
-Set subtitles input character encoding. @code{subtitles} filter only. Only
-useful if not UTF-8.
+Set subtitles input character encoding. Only useful if not UTF-8.
@item stream_index, si
-Set subtitles stream index. @code{subtitles} filter only.
+Set subtitles stream index.
@item force_style
Override default style or script info parameters of the subtitles. It accepts a
--
2.17.1
___
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 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video.
This avoids having to rewrite the subtitle file in order to display
subtitles correctly when input is seeked (-ss).
Also handy for minor subtitle timing corrections without rewriting the
subtitles file.
Signed-off-by: Manolis Stamatogiannakis
---
doc/filters.texi | 11
libavfilter/vf_subtitles.c | 55 +-
2 files changed, 59 insertions(+), 7 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index d2b8feb14b..a8af563551 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -17936,6 +17936,9 @@ Common @ref{subtitles}/@ref{ass} filter options:
@item filename, f
Set the filename of the subtitle file to read. It must be specified.
+@item shift
+Shift subtitles timings by the specified amount.
+
@item original_size
Specify the size of the original video, the video for which the ASS file
was composed. For the syntax of this option, check the
@@ -17949,6 +17952,9 @@ These fonts will be used in addition to whatever the
font provider uses.
@item alpha
Process alpha channel, by default alpha channel is untouched.
+
+@item shift
+Shift subtitles timings by the specified amount.
@end table
Additional options for @ref{subtitles} filter:
@@ -17995,6 +18001,11 @@ To make the subtitles stream from @file{sub.srt}
appear in 80% transparent blue
subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour=&HCCFF'
@end example
+To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use:
+@example
+subtitles=filename=sub.srt:shift='-20\:20'
+@end example
+
@section super2xsai
Scale the input by 2x and smooth using the Super2xSaI (Scale and
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 1bd42391e0..af1bc9cd18 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -52,6 +52,7 @@ typedef struct AssContext {
char *filename;
char *fontsdir;
char *charenc;
+int64_t shift;
char *force_style;
int stream_index;
int alpha;
@@ -66,11 +67,12 @@ typedef struct AssContext {
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
#define COMMON_OPTIONS \
-{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0,
FLAGS }, \
-{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \
-{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"filename", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"f", "set the filename of file to read",
OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"original_size", "set the size of the original video (used to scale
fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0,
0, FLAGS }, \
+{"fontsdir", "set the directory containing the fonts to read",
OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0,
0, FLAGS }, \
+{"alpha", "enable processing of alpha channel",
OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0,
1, FLAGS }, \
+{"shift", "shift subtitles timing",
OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN,
INT64_MAX, FLAGS }, \
/* libass supports a log level ranging from 0 to 7 */
static const int ass_libavfilter_log_level_map[] = {
@@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx)
return AVERROR(EINVAL);
}
+if (ass->shift != 0) {
+ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1,
1000));
+av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n",
ass->shift/1000.0);
+}
+
ass->library = ass_library_init();
if (!ass->library) {
av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
@@ -227,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass);
static av_cold int init_ass(AVFilterContext *ctx)
{
+int eid, nskip;
+ASS_Event *event;
AssContext *ass = ctx->priv;
int ret = init(ctx);
@@ -243,6 +252,25 @@ static av_cold int init_ass(AVFilterContext *ctx)
Re: [FFmpeg-devel] patch submission questions
On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong wrote: > > You can use `git format-patch -v -n` to get patches like [PATCH > v2 1/20]. See git-format-patch documentation for more details. > > That didn't quite work. I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org HEAD~2..HEAD" and "git send-email -2". But the -v argument was dropped and the end results on patchwork were again: [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters. [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter options. Manolis ___ 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] Project orientation
On Sun, Jul 05, 2020 at 08:13:25PM +0200, Manolis Stamatogiannakis wrote: > As a fresh contributor, setting up git send-email was a hassle, but > not an insurmountable obstacle. Speaking only for myself, having sent a single-digit number of patches to FFmpeg ever: Setting up git send-email was not a big turnoff. Having your patch being not responded to (whether being forgotten, not found interesting enough, or whatever other reason) was. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ 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 submission questions
On Sun, 05. Jul 20:34, Manolis Stamatogiannakis wrote: > On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong > wrote: > > > > > You can use `git format-patch -v -n` to get patches like [PATCH > > v2 1/20]. See git-format-patch documentation for more details. > > > > > That didn't quite work. > > I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org > HEAD~2..HEAD" and "git send-email -2". > > But the -v argument was dropped and the end results on patchwork were again: > [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for > subtitles/ass filters. > [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter > options. > git send-email -v2 HEAD~ # works for me git shows are draft of the email before sending. Check that the subject line contains the version number (or you can send the patch to yourself) For patchwork, you can create an account and set your old patches as 'Superseded'. I also periodically run a script that sets Superseded and Accepted labels. -- Andriy ___ 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] ABI break in 4.3
On Sunday 2020-07-05 19:21, James Almer wrote: >> 8 files changed, 1163 insertions(+), 44 deletions(-) > >The list of functions is incomplete. After a quick look, you're for >example missing av_map_videotoolbox_format_* in libavutil.[..] >This is another issue that i don't know if you considered, and that's >the fact currently some symbols may not present depending on configure >time options, build environment, and target system. Videotoolbox is of >course not available for you unless you're targeting OSX. An alternative to building an initial list from `nm` is to visually work through the function declarations in .h files. This also requires knowing which of those get installed to /usr/include and which constitute private headers. I am sure you are more capable of knowing which ones are which; I have only worked with ffmpeg source for a day. ___ 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] Project orientation
On Sun, 5 Jul 2020, Kieran Kunhya wrote: People aren't using it because people don't use MPEG-2. > There is no maintenance to be done on a format that's 25 years old, do > you want me to randomly change cosmetics to make you feel happy? If you'd get off your high horse for once, that would make me feel happy. Please let me know what "maintenance" you'd like me to do on x262? I don't want you to do anything. But for example some code changes need to be merged back from x264 if somebody wants to make x262 compile using the latest ffmpeg. I don't think I told anybody to implement the missing features for free. But I do believe that payed or free development has a higher chance of happening if existing code is already available in a popular package/project. And yes, I believe that some "maintenance burden" should be accepted by the base project to give more chance to further advancement, payed or free. It's not your pejorative to say that x264 developers have to accept and maintain a merge back of x262. I am not saying they have to. I am saying that x262 would have benefited from it and the maintenance burden as I estimate it would not have been unsurmountable for x264. Also this is quite complex now owing the combined 8/10-bit single binary. Furthermore any change to H.264 they would now have to test and maintain for MPEG-2. You seem to imply that this work is quite simple which is easy for you to say when you are the one not doing it. I honestly don't know how much work it is. I assumed it is not too much, especially if somebody is familiar with the codebase. Now that the trees have diverged, obviously it has become harder. But dealing with the changes is usually easier if the feature is in-tree already. Do you plan to do this work? I don't know enough about x262/x264 to do this with reasonable amount of work. Do you think there is a chance of this happening if I post a bounty or get a sponsorship? Regards, 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] ABI break in 4.3
On 7/5/2020 3:59 PM, Jan Engelhardt wrote: > > On Sunday 2020-07-05 19:21, James Almer wrote: >>> 8 files changed, 1163 insertions(+), 44 deletions(-) >> >> The list of functions is incomplete. After a quick look, you're for >> example missing av_map_videotoolbox_format_* in libavutil.[..] >> This is another issue that i don't know if you considered, and that's >> the fact currently some symbols may not present depending on configure >> time options, build environment, and target system. Videotoolbox is of >> course not available for you unless you're targeting OSX. > > An alternative to building an initial list from `nm` is to > visually work through the function declarations in .h files. > This also requires knowing which of those get installed to /usr/include > and which constitute private headers. The list of installed headers is $(HEADERS) in each of the project's library specific Makefiles. > I am sure you are more capable of knowing which ones are > which; I have only worked with ffmpeg source for a day. My intention was to state that ultimately it may not be a simple task to get a fixed, non build time generated .ver file for this purpose. The fact some symbols (at least currently) can be present or not may play against it. ___ 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] Project orientation
> x264 is practically feature complete, but x262 still miss some things, > like 4:2:2 interlaced. Sure, x262 can work well enough for some use cases, > but it is still not packaged in e.g. Ubuntu, so users are stuck with the > - in some ways - inferior mpeg2 encoder of ffmpeg. > > The point I am trying to make is that you and some other people made a > fast and modern mpeg2 encoder, in some ways superior to existing open > source alternatives, but very few people is using it because it was not > merged to a bigger/more popular project like x264 or ffmpeg. So it > receives no mainteance, no distribution support, no user base and > ultimately no further development. Or at least that is how I see it. People aren't using it because people don't use MPEG-2. There is no maintenance to be done on a format that's 25 years old, do you want me to randomly change cosmetics to make you feel happy? I know of people using it 24/7 for many years and have had no issues. Have you opened bug reports on x264-devel about the issues you see? MPEG-2 4:2:2 interlaced users are infinitesimally small and professional users. If you want your pet feature to work, lo and behold you have to work on it or pay someone to work on it. If you want people to work on it out of nowhere then you are part of the Free Rider Problem: https://en.wikipedia.org/wiki/Free-rider_problem x264 is heavily used but the changes these days are very minor in spite of all these things. So your argument is incorrect. Kieran ___ 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 v06 1/5] KMSGrab: getfb2 format_modifier if user doesnt specify
Hi
Don't apply this patch, I will try and use ioctl directly instead of using
xf86's GetFB2.
Based on the reported compile issue with initialising global const for the
fbtile patch when using older versions of gcc, I was checking the same
using Ubuntu 16.04 setup, and realised that the older xf86drmMode.h in it
doesn't provide GetFB2.
On Sun, 5 Jul, 2020, 00:51 Lynne, wrote:
> Jul 4, 2020, 14:17 by hanish...@gmail.com:
>
> > If user doesnt specify a format_modifier explicitly, then use GetFB2
> > to identify the format_modifier of the framebuffer being grabbed.
> > ---
> > Changelog | 1 +
> > libavdevice/kmsgrab.c | 22 +-
> > 2 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/Changelog b/Changelog
> > index a60e7d2eb8..3881587caa 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to
> youngest within each release,
> > releases are sorted from youngest to oldest.
> >
> > version :
> > +- kmsgrab GetFB2 format_modifier, if user doesnt specify
> > - AudioToolbox output device
> > - MacCaption demuxer
> >
> > diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
> > index d0de774871..10ed707e60 100644
> > --- a/libavdevice/kmsgrab.c
> > +++ b/libavdevice/kmsgrab.c
> > @@ -239,6 +239,7 @@ static av_cold int
> kmsgrab_read_header(AVFormatContext *avctx)
> > drmModePlaneRes *plane_res = NULL;
> > drmModePlane *plane = NULL;
> > drmModeFB *fb = NULL;
> > +drmModeFB2 *fb2 = NULL;
> > AVStream *stream;
> > int err, i;
> >
> > @@ -364,6 +365,23 @@ static av_cold int
> kmsgrab_read_header(AVFormatContext *avctx)
> > goto fail;
> > }
> >
> > +fb2 = drmModeGetFB2(ctx->hwctx->fd, plane->fb_id);
> > +if (!fb2) {
> > +err = errno;
> > +av_log(avctx, AV_LOG_ERROR, "Failed to get "
> > + "framebuffer2 %"PRIu32": %s.\n",
> > + plane->fb_id, strerror(err));
> > +err = AVERROR(err);
> > +goto fail;
> > +}
> > +
> > +av_log(avctx, AV_LOG_INFO, "Template framebuffer2 is %"PRIu32": "
> > + "%"PRIu32"x%"PRIu32", pixel_format: 0x%"PRIx32",
> format_modifier: 0x%"PRIx64".\n",
> > + fb2->fb_id, fb2->width, fb2->height, fb2->pixel_format,
> fb2->modifier);
> > +
> > +if (ctx->drm_format_modifier == DRM_FORMAT_MOD_INVALID)
> > +ctx->drm_format_modifier = fb2->modifier;
> > +
> > stream = avformat_new_stream(avctx, NULL);
> > if (!stream) {
> > err = AVERROR(ENOMEM);
> > @@ -408,6 +426,8 @@ fail:
> > drmModeFreePlane(plane);
> > if (fb)
> > drmModeFreeFB(fb);
> > +if (fb2)
> > +drmModeFreeFB2(fb2);
> >
> > return err;
> > }
> > @@ -433,7 +453,7 @@ static const AVOption options[] = {
> > { .i64 = AV_PIX_FMT_BGR0 }, 0, UINT32_MAX, FLAGS },
> > { "format_modifier", "DRM format modifier for framebuffer",
> > OFFSET(drm_format_modifier), AV_OPT_TYPE_INT64,
> > - { .i64 = DRM_FORMAT_MOD_NONE }, 0, INT64_MAX, FLAGS },
> > + { .i64 = DRM_FORMAT_MOD_INVALID}, 0, INT64_MAX, FLAGS },
> > { "crtc_id", "CRTC ID to define capture source",
> > OFFSET(source_crtc), AV_OPT_TYPE_INT64,
> > { .i64 = 0 }, 0, UINT32_MAX, FLAGS },
> > --
> > 2.25.1
> >
> > ___
> > 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".
> >
>
> This one looks fine to me, but Mark Thompson should check this one too.
>
> ___
> 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 v06 3/5] hwcontext_drm detile non linear layout, if possible
Hi Lynne, Am confused? Please look at this patch again. I directly detile from mmaped hardware framebuffer into specified output buffer. Only if the tile layout format is not supported, it falls back to the original frame copy. I am assuming you have misread the patch. On Sun, 5 Jul, 2020, 00:58 Lynne, wrote: > Jul 4, 2020, 14:17 by hanish...@gmail.com: > > > If the framebuffer is a tiled layout, use the fbtile helper routines > > to try and detile it into linear layout, if supported by fbtile. > > > > It uses the format_modifier associated with the framebuffer to decide > > whether to apply detiling or not and inturn which specific detiling > > to apply. > > > > If user is using kmsgrab, they will have to use -format_modifer option > > of kmsgrab to force a specific detile logic, in case they dont want to > > use the original format_modifier related detiling. Or they could even > > use -format_modifier 0 to make hwcontext_drm bypass this detiling. > > --- > > Changelog | 1 + > > libavutil/hwcontext_drm.c | 32 ++-- > > 2 files changed, 31 insertions(+), 2 deletions(-) > > > > diff --git a/Changelog b/Changelog > > index 3881587caa..b6a4ad1b34 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to > youngest within each release, > > releases are sorted from youngest to oldest. > > > > version : > > +- hwcontext_drm detiles non linear layouts, if possible > > - kmsgrab GetFB2 format_modifier, if user doesnt specify > > - AudioToolbox output device > > - MacCaption demuxer > > diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c > > index 32cbde82eb..bd74b3f13d 100644 > > --- a/libavutil/hwcontext_drm.c > > +++ b/libavutil/hwcontext_drm.c > > @@ -21,6 +21,7 @@ > > #include > > > > #include > > +#include > > #include > > > > #include "avassert.h" > > @@ -28,6 +29,7 @@ > > #include "hwcontext_drm.h" > > #include "hwcontext_internal.h" > > #include "imgutils.h" > > +#include "fbtile.h" > > > > > > static void drm_device_free(AVHWDeviceContext *hwdev) > > @@ -185,6 +187,32 @@ static int > drm_transfer_get_formats(AVHWFramesContext *ctx, > > return 0; > > } > > > > +// Can be overridden during compiling, if required. > > +#ifndef HWCTXDRM_SYNCRELATED_FORMATMODIFIER > > +#define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 1 > > +#endif > > > > This is also not acceptable, I'm afraid. We don't change behavior with > compile-time checks, > and those are really not the correct way to do it. > I get why you want to be able to dump tiled video but please understand, > we can't accept > this as-is. > Instead of looking for ways to make hacks part of the code base why not > look into improving > the performance of detiling? There's so much more to do than copying a > frame and detiling it. > You could copy and detile in-place, or you can avoid copying entirely by > just directly detiling > out of place to the destination frame. > > ___ > 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] Project orientation
> > People aren't using it because people don't use MPEG-2. > > There is no maintenance to be done on a format that's 25 years old, do > > you want me to randomly change cosmetics to make you feel happy? > > If you'd get off your high horse for once, that would make me feel happy. Please let me know what "maintenance" you'd like me to do on x262? > I don't think I told anybody to implement the missing features for free. > But I do believe that payed or free development has a higher chance of > happening if existing code is already available in a popular > package/project. And yes, I believe that some "maintenance burden" should > be accepted by the base project to give more chance to further > advancement, payed or free. It's not your pejorative to say that x264 developers have to accept and maintain a merge back of x262. Also this is quite complex now owing the combined 8/10-bit single binary. Furthermore any change to H.264 they would now have to test and maintain for MPEG-2. You seem to imply that this work is quite simple which is easy for you to say when you are the one not doing it. Do you plan to do this work? Kieran ___ 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 v06 4/5] hwdownload detile framebuffer, if requested by user
Hi Lynne, On Sun, 5 Jul, 2020, 00:59 Lynne, wrote: > Jul 4, 2020, 14:17 by hanish...@gmail.com: > > > Added logic to support detiling of framebuffer. > > > > By default this is disabled. Only if requested by the user, the > > logic will be triggered. > > > > It uses the fbtile helper routines to do the detiling. Currently > > 32bit RGB pixel format based framebuffers are supported. > > > > If the underlying hardware context provides linear layouts, then > > nothing is done. Only if the underlying hardware context generates > > tiled layout, then user can use this to detile, where possible. > > > > ./ffmpeg -f kmsgrab -i - -vf hwdownload=1,format=bgr0 out.mp4 > > > > Not acceptable for the reasons stated in the hwcontext patch. > Besides, allowing users to detile non-tiled images is a really, _really_ > bad idea. > As already responded, in case of the hwcontext patch, I take care of avoiding the frame copy, so that it's efficient, where possible. While this patch is different, as mentioned in this patch note. The idea here is to provide a fall back option for detiling such that it's independent of the underlying hwcontext, in case if such a situation arises in future. And Or Parallely it allows the user to force a detile, if they so desire explicitly. As this is not automatic, but has to be triggered by the user, It doesn't add any overhead by default, but gives the flexibility in case the user so desires. I do understand that there is a fundamental difference of opinion wrt whether to give this additional flexibility to user or not between my thinking and yours. I wanted to just show that it can be done in a simple and relatively clean way without overhead, when not desired. However It's finally your+any additional ffmpeg teams call. > ___ > 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] Project orientation
On Sun, Jul 05, 2020 at 08:42:13PM +0200, Steinar H. Gunderson wrote: > On Sun, Jul 05, 2020 at 08:13:25PM +0200, Manolis Stamatogiannakis wrote: > > As a fresh contributor, setting up git send-email was a hassle, but > > not an insurmountable obstacle. > > Speaking only for myself, having sent a single-digit number of patches > to FFmpeg ever: Setting up git send-email was not a big turnoff. > Having your > patch being not responded to (whether being forgotten, not found interesting > enough, or whatever other reason) was. At least for me the reason to not review a patch is often simply time. But i agree the amount of not reviewed patches is a problem. How can we solve this ? From a mathematical point of view either more reviews must happen or fewer patches have to be sent. Fewer patches would only make sense if they are replaced by selfreview and direct commits. This could reduce the load on reviewers. We did this in the past and we had fewer non reviewed patches back then. I do not know what peoples oppinion is about this but there has been opposition to this long ago when it was commonly done. The second thing is more reviews. That can happen by * More reviewers * More reviews per reviewer * Less work per review To Achieve this, we could try to * attract more developers doing reviews, i have generally suggested contributors to help review other peoples patches. Maybe i should take a step back and ask developers to ask developers to do this instead. It is a way out of this problem * make people have a burning desire to review patches. I understand this would work very well but iam not sure how to achieve this * pay developers to do reviews, i think we do not yet have the funds for this as reviews take alot of time and thus this would not be cheap Also to make reviews less work * Code documentation should be improved * Testcases in fate should be mandatory for newly added "parts", this allows easy testing of changes and allows filtering out some bugs automatically Some simple suggestion * If you submmit a patch and its reviewed by someone, please pick some other patch by someone in an area you reasonably understand and review it (If everyone does this we have no lack of reviewers anymore) Anyway i dont think the unreviewed patches amount is a unsolveable problem Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. 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] Project orientation
On Sun, Jul 5, 2020 at 9:10 PM Marton Balint wrote: > I don't know enough about x262/x264 to do this with reasonable amount of > work. Do you think there is a chance of this happening if I post a bounty > or get a sponsorship? x264 is an H.264/AVC encoder and as such an MPEG-2 encoder is not in scope to be included. Just because a use case exists doesn't mean it's a good idea to accept every feature under the sun upstream. Having a separate fork for that is perfectly reasonable. ___ 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] Project orientation
Going back to the original point in hand. Many patches aren't getting reviewed and pushed any more. In part this is because in 2020 whether we like it or not mailing lists are not the way to do Git based development. The kernel is the exception to the rule, as Linus says it has a whole load of grey-bearded system maintainers who are paid full time to work on it. For new contributors git send-email is annoying. For people wanting to push, the .mbox format is annoying, Gmail doesn't support it any more. And you can't get new contributors to start using CLI based email clients or run their own mail server, that's not going to happen. A solution like Gitlab is the only way forward. It has worked well for dav1d, it can run regression tests on all platforms for all commits: https://code.videolan.org/videolan/dav1d Merges are done with one push of a button. Yes, the branch sprawl is not great but it's better than now. It has inline patch reviews which are nice. Whether we like it or not web interfaces are the way 95% of the world does Git and we have to move with the times. Kieran On Sun, Jul 5, 2020 at 5:38 PM Kieran Kunhya wrote: > > > > People aren't using it because people don't use MPEG-2. > > > There is no maintenance to be done on a format that's 25 years old, do > > > you want me to randomly change cosmetics to make you feel happy? > > > > If you'd get off your high horse for once, that would make me feel happy. > > Please let me know what "maintenance" you'd like me to do on x262? > > > I don't think I told anybody to implement the missing features for free. > > But I do believe that payed or free development has a higher chance of > > happening if existing code is already available in a popular > > package/project. And yes, I believe that some "maintenance burden" should > > be accepted by the base project to give more chance to further > > advancement, payed or free. > > It's not your pejorative to say that x264 developers have to accept > and maintain a merge back of x262. > Also this is quite complex now owing the combined 8/10-bit single > binary. Furthermore any change to H.264 they would now have to test > and maintain for MPEG-2. > You seem to imply that this work is quite simple which is easy for you > to say when you are the one not doing it. > Do you plan to do this work? > > Kieran ___ 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] ABI break in 4.3
On Sun, Jul 05, 2020 at 05:08:51PM +0200, Tomas Härdin wrote:
> sön 2020-07-05 klockan 14:10 +0200 skrev Jan Engelhardt:
> > On Sunday 2020-07-05 13:39, Tomas Härdin wrote:
> > > > > Downgrading to a .so file with a lower minor version number than
> > > > > the program is built against can never be expected to work. Else
> > > > > we couldn't add new functions without a major bump.
> > > >
> > > > It requires the use ELF symbol versions -- which ffmpeg fails to
> > > > do properly.[...]
> > >
> > > This is a fair point. I didn't actually know the loader can do stuff
> > > like this, sounds super handy. How hard would it be to get that going?
> >
> > Change libavcodec.v to
> >
> > LIBAVCODEC_58 {
> > global:
> > av_foo;
> > av_bar;
> > av_whatever_else_there_is;...
> > local:
> > *;
> > };
> > LIBAVCODEC_58.91 {
> > global:
> > avpriv_mpeg4audio_get_config2;
> > } LIBAVCODEC_58;
> > LIBAVCODEC_58.123 { /* future example */
> > global:
> > avblahblah;
> > } LIBAVCODEC_58.91;
> >
> > Repeat likewise for other .v files. The file is no longer a template. The
> > automatic substitution of "_MAJOR" by the build system needs to cease.
> > Version
> > numbers in the file are supposed to be fixed (among the set of all .so files
> > that share a SONAME).
>
> Interesting. This also makes what's changed between versions more
> explicit. Can this be checked by machine as well? Like having a post-
> receive hook that makes sure the .v files are consistent, or maybe have
> FATE check it somehow.
its possible to do some sanity checks at the git level that would catch some
issues.
(for example you can detect changed version numbers and reject if that doesnt
also update APIchanges
and then parse the change to APIchanges and the change to the .v file and
check if the same functions and versions are listed.)
At the fate level it should be easier to detect the actual functions in the
object files and compare this to the .v files
so its all doable in theory, but at that point the question arises, can we
maybe generate this automatically from the APIchanges file if we decide to
do that.
APIchanges should list all added functions and the version numbers.
>
> This probably needs to be passed through the technical committee. But I
> don't think it'll break the ABI like Timo suggests, if we bump minor at
> the same time.
the technical committee is for disagreement arbitraration.
ATM i dont think we have any disagreement
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
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] Project orientation
On Sun, Jul 05, 2020 at 10:50:14PM +0200, Michael Niedermayer wrote: > At least for me the reason to not review a patch is often simply > time. > But i agree the amount of not reviewed patches is a problem. > > [...] > > The second thing is more reviews. > That can happen by > * More reviewers > * More reviews per reviewer > * Less work per review I think this depends on whether the problem is time at any given instant, or over a longer period of time. If the problem is “people don't have time right now”, what is needed is a way to make sure patches are not forgotten until the point where someone has time (so, effectively more reviews per reviewer). If the problem is that reviewers are generally overworked, one needs more reviewers or less work per review, as you say. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ 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] Project orientation
On Sun, Jul 5, 2020 at 6:01 PM Kieran Kunhya wrote: > > Going back to the original point in hand. > Many patches aren't getting reviewed and pushed any more. > > In part this is because in 2020 whether we like it or not mailing > lists are not the way to do Git based development. > The kernel is the exception to the rule, as Linus says it has a whole > load of grey-bearded system maintainers who are paid full time to work > on it. > > For new contributors git send-email is annoying. For people wanting to > push, the .mbox format is annoying, Gmail doesn't support it any more. > And you can't get new contributors to start using CLI based email > clients or run their own mail server, that's not going to happen. > > A solution like Gitlab is the only way forward. It has worked well for > dav1d, it can run regression tests on all platforms for all commits: > https://code.videolan.org/videolan/dav1d > > Merges are done with one push of a button. Yes, the branch sprawl is > not great but it's better than now. > It has inline patch reviews which are nice. > > Whether we like it or not web interfaces are the way 95% of the world > does Git and we have to move with the times. Not my intention to top post but gmail hides quoted text. Forgot to add that git send-email is quite complex to setup now without your own mail server. This also restricts our ability to add new developers. Kieran ___ 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] Project orientation
On Sun, Jul 5, 2020, at 14:28, Marton Balint wrote: > having AMQP/ZMQ protocol support is much more useful then Lego Racers > demuxer... You are missing the point here: Lego Racers demuxer is in the scope of "play everything under the sun" that the FFmpeg project is, while AMQP/ZMQ is not. The issue is not usefulness, but correctly defining the scope of the project (aka the orienttation). If you don't, you will integrate a webserver, an email sender, a full webRTC stack with p2p and a coffee controller. Some codecs and formats are not useful, sure, but they are in the scope, and there is little debate about it. But custom protocols or complex filters that bring huge dependencies are very debatable, and I doubt they are the consensus. Also in this example, noone is telling them to fork, just to use the API to register their custom protocol. -- Jean-Baptiste Kempf - President +33 672 704 734 ___ 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] Project orientation
On Sun, Jul 5, 2020, at 18:25, Marton Balint wrote: > > People aren't using it because people don't use MPEG-2. > > There is no maintenance to be done on a format that's 25 years old, do > > you want me to randomly change cosmetics to make you feel happy? > > If you'd get off your high horse for once, that would make me feel happy. This kind of tone is unwelcome. -- Jean-Baptiste Kempf - President +33 672 704 734 ___ 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 v1 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
The section has been expanded to outline how to manage patch revisions.
---
doc/developer.texi | 37 ++---
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index b33cab0fc7..dec27cb509 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -491,18 +491,25 @@ as base64-encoded attachments, so your patch is not
trashed during
transmission. Also ensure the correct mime type is used
(text/x-diff or text/x-patch or at least text/plain) and that only one
patch is inline or attached per mail.
-You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show
up, its mime type
-likely was wrong.
+You can check the most recently received patches on
+@url{https://patchwork.ffmpeg.org, patchwork}.
+If your patch does not show up, its mime type likely was wrong.
-Your patch will be reviewed on the mailing list. You will likely be asked
+Your patch will be reviewed on the mailing list. Give us a few days to react.
+But if some time passes without reaction, send a reminder by email.
+Your patch should eventually be dealt with. However, you will likely be asked
to make some changes and are expected to send in an improved version that
incorporates the requests from the review. This process may go through
several iterations. Once your patch is deemed good enough, some developer
will pick it up and commit it to the official FFmpeg tree.
-Give us a few days to react. But if some time passes without reaction,
-send a reminder by email. Your patch should eventually be dealt with.
-
+When preparing an updated version of you patch, make sure you increment
+its @emph{roll-counter}. This is achieved by adding a @code{-v } argument
+to @code{git format-patch}/@code{git send-email} commands.
+Additionally, it is recommended to register for a
+@uref{https://patchwork.ffmpeg.org, patchwork account}.
+This will allow you to mark previous version of your patches as "Superseded",
+and reduce the chance of someone spending time to review a stale patch.
@chapter New codecs or formats checklist
@@ -563,7 +570,19 @@ Did you make sure it compiles standalone, i.e. with
Does @code{make fate} pass with the patch applied?
@item
-Was the patch generated with git format-patch or send-email?
+Was the patch generated with @code{git format-patch} or @code{git send-email}?
+
+@item
+If you are submitting a revised patch, did you increment the roll-counter
+with @code{-v }?
+
+@item
+If you are submitting a revised patch, did you marked previous versions of
+the patch as "Superseded" on @uref{https://patchwork.ffmpeg.org/, patchwork}?
+
+@item
+Are you subscribed to ffmpeg-devel?
+(the list is subscribers only due to spam)
@item
Did you sign-off your patch? (@code{git commit -s})
@@ -576,10 +595,6 @@ Did you provide a clear git commit log message?
@item
Is the patch against latest FFmpeg git master branch?
-@item
-Are you subscribed to ffmpeg-devel?
-(the list is subscribers only due to spam)
-
@item
Have you checked that the changes are minimal, so that the same cannot be
achieved with a smaller patch and/or simpler final code?
--
2.17.1
___
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 v1 3/3] doc/developer.texi: Swapped patch checklist and new codec/format checklist.
Adding a new codec/format should be more rare, so it makes sense
to come after the detailed patch submission checklist.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 105 ++---
1 file changed, 52 insertions(+), 53 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index 6d4f6afcf9..cecb10fed1 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -529,59 +529,6 @@ Additionally, it is recommended to register for a
This will allow you to mark previous version of your patches as "Superseded",
and reduce the chance of someone spending time to review a stale patch.
-@anchor{new codec format checklist}
-@section New codecs or formats checklist
-
-@enumerate
-@item
-Did you use av_cold for codec initialization and close functions?
-
-@item
-Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or
-AVInputFormat/AVOutputFormat struct?
-
-@item
-Did you bump the minor version number (and reset the micro version
-number) in @file{libavcodec/version.h} or @file{libavformat/version.h}?
-
-@item
-Did you register it in @file{allcodecs.c} or @file{allformats.c}?
-
-@item
-Did you add the AVCodecID to @file{avcodec.h}?
-When adding new codec IDs, also add an entry to the codec descriptor
-list in @file{libavcodec/codec_desc.c}.
-
-@item
-If it has a FourCC, did you add it to @file{libavformat/riff.c},
-even if it is only a decoder?
-
-@item
-Did you add a rule to compile the appropriate files in the Makefile?
-Remember to do this even if you're just adding a format to a file that is
-already being compiled by some other rule, like a raw demuxer.
-
-@item
-Did you add an entry to the table of supported formats or codecs in
-@file{doc/general.texi}?
-
-@item
-Did you add an entry in the Changelog?
-
-@item
-If it depends on a parser or a library, did you add that dependency in
-configure?
-
-@item
-Did you @code{git add} the appropriate files before committing?
-
-@item
-Did you make sure it compiles standalone, i.e. with
-@code{configure --disable-everything --enable-decoder=foo}
-(or @code{--enable-demuxer} or whatever your component is)?
-@end enumerate
-
-
@anchor{patch submission checklist}
@section Patch submission checklist
@@ -708,6 +655,58 @@ Test your code with valgrind and or Address Sanitizer to
ensure it's free
of leaks, out of array accesses, etc.
@end enumerate
+@anchor{new codec format checklist}
+@section New codecs or formats checklist
+
+@enumerate
+@item
+Did you use av_cold for codec initialization and close functions?
+
+@item
+Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or
+AVInputFormat/AVOutputFormat struct?
+
+@item
+Did you bump the minor version number (and reset the micro version
+number) in @file{libavcodec/version.h} or @file{libavformat/version.h}?
+
+@item
+Did you register it in @file{allcodecs.c} or @file{allformats.c}?
+
+@item
+Did you add the AVCodecID to @file{avcodec.h}?
+When adding new codec IDs, also add an entry to the codec descriptor
+list in @file{libavcodec/codec_desc.c}.
+
+@item
+If it has a FourCC, did you add it to @file{libavformat/riff.c},
+even if it is only a decoder?
+
+@item
+Did you add a rule to compile the appropriate files in the Makefile?
+Remember to do this even if you're just adding a format to a file that is
+already being compiled by some other rule, like a raw demuxer.
+
+@item
+Did you add an entry to the table of supported formats or codecs in
+@file{doc/general.texi}?
+
+@item
+Did you add an entry in the Changelog?
+
+@item
+If it depends on a parser or a library, did you add that dependency in
+configure?
+
+@item
+Did you @code{git add} the appropriate files before committing?
+
+@item
+Did you make sure it compiles standalone, i.e. with
+@code{configure --disable-everything --enable-decoder=foo}
+(or @code{--enable-demuxer} or whatever your component is)?
+@end enumerate
+
@chapter Patch review process
All patches posted to ffmpeg-devel will be reviewed, unless they contain a
--
2.17.1
___
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 v1 2/3] doc/developer.texi: Restructured "Submitting patches" section.
- Main text split to two sections.
- Detailed checklist for new codecs or formats demoted to section.
- Detailed checklist for patch submission demoted to section.
Signed-off-by: Manolis Stamatogiannakis
---
doc/developer.texi | 64 +++---
1 file changed, 44 insertions(+), 20 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index dec27cb509..6d4f6afcf9 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -457,31 +457,49 @@ Finally, keep in mind the immortal words of Bill and Ted,
@anchor{Submitting patches}
@chapter Submitting patches
-First, read the @ref{Coding Rules} above if you did not yet, in particular
+@anchor{patch guidelines}
+@section Guidelines for preparing a patch
+
+The @strong{absolute minimum} you have to do before submitting a patch are the
+following:
+
+@enumerate
+@item Carefully read the @ref{Coding Rules} above if you did not yet, in
particular
the rules regarding patch submission.
-When you submit your patch, please use @code{git format-patch} or
-@code{git send-email}. We cannot read other diffs :-).
+@item Make sure your commit messages accurately describe the changes made
+(e.g. 'replaces lrint by lrintf') and why these changes are made (e.g.
+'*BSD isn't C99 compliant and has no lrint()').
-Also please do not submit a patch which contains several unrelated changes.
-Split it into separate, self-contained pieces. This does not mean splitting
-file by file. Instead, make the patch as small as possible while still
-keeping it as a logical unit that contains an individual change, even
-if it spans multiple files. This makes reviewing your patches much easier
-for us and greatly increases your chances of getting your patch applied.
+@item Make sure you use @code{git format-patch} or @code{git send-email} to
prepare
+your patch. We cannot read other diffs :-).
+
+@item Run the @ref{Regression tests, regression tests} before submitting a
patch
+in order to verify it does not cause unexpected problems.
-Use the patcheck tool of FFmpeg to check your patch.
-The tool is located in the tools directory.
+@item If you send your patches with an external email client
+(i.e. not @code{git send-email}), make sure to send each patch as a separate
+email. Do not attach several patches to the same email!
-Run the @ref{Regression tests} before submitting a patch in order to verify
-it does not cause unexpected problems.
+@item Do not submit a patch which contains several unrelated changes.
+@end enumerate
+
+Additionally, it is also important that the commits comprising a patch
+are logically self-contained. I.e. each commit should be as small as
+possible while still containing a meaningful individual change.
+Commits spanning multiple files are perfectly fine, as long as the
+commit can be seen as a single logical unit.
-It also helps quite a bit if you tell us what the patch does (for example
-'replaces lrint by lrintf'), and why (for example '*BSD isn't C99 compliant
-and has no lrint()')
+Following these guidelines makes reviewing your patches much easier
+for us and greatly increases your chances of getting your patch applied.
+To further reduce the chance that you will need to revise your patch,
+it is also recommended to go through the detailed
+@ref{patch submission checklist, patch} and
+@ref{new codec format checklist, new codec or format}
+checklists.
-Also please if you send several patches, send each patch as a separate mail,
-do not attach several unrelated patches to the same mail.
+@anchor{patch submission process}
+@section Patch submission and revision process
Patches should be posted to the
@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel}
@@ -511,7 +529,8 @@ Additionally, it is recommended to register for a
This will allow you to mark previous version of your patches as "Superseded",
and reduce the chance of someone spending time to review a stale patch.
-@chapter New codecs or formats checklist
+@anchor{new codec format checklist}
+@section New codecs or formats checklist
@enumerate
@item
@@ -563,7 +582,8 @@ Did you make sure it compiles standalone, i.e. with
@end enumerate
-@chapter Patch submission checklist
+@anchor{patch submission checklist}
+@section Patch submission checklist
@enumerate
@item
@@ -592,6 +612,10 @@ of @dfn{sign-off}.
@item
Did you provide a clear git commit log message?
+@item
+Did you use the @code{patcheck} tool of FFmpeg to check your patch
+for common issues? E.g. @code{tools/patcheck *.patch}.
+
@item
Is the patch against latest FFmpeg git master branch?
--
2.17.1
___
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] Project orientation
On Sun, Jul 5, 2020, at 22:50, Michael Niedermayer wrote: > > Having your > > patch being not responded to (whether being forgotten, not found interesting > > enough, or whatever other reason) was. > > At least for me the reason to not review a patch is often simply > time. > But i agree the amount of not reviewed patches is a problem. Yes. > How can we solve this ? With tools and organization. Today, any patchset that has several patches (like the 36 ones for VP9 of the other day) or multiple revisions gets everyone a ton of emails. When you are at the revision 3 or 7, you completely lost track, unless you know exactly what to look for. Seeing the difference from the 2 revisions is very hard. So either you spend a lot of time understanding and re-reading or you drop it. So, either inefficiency for the reviewer, and therefore less reviews, or forgotten patches. Today the patchwork has 64 pages of patches... Noone human can know all that is happening. So either you split the mailing list and patches per library (different for libavcodec, format), with submaintainers for each library, basically, like the Linux Kernel does. Or you move to github/gitlab which gives you by default a tracking of the MR that got merged or not, where you can see quickly the differences between 2 revisions of a patchset and where you can use tags to mark the merge requests, where the CI is automatic, so you don't have to check manually that the code compiles or that FATE is not broken, etc... -- Jean-Baptiste Kempf - President +33 672 704 734 ___ 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] ABI break in 4.3
On Sunday 2020-07-05 23:17, Michael Niedermayer wrote: > >so its all doable in theory, but at that point the question arises, can we >maybe generate this automatically from the APIchanges file if we decide to >do that. >APIchanges should list all added functions and the version numbers. APIchanges would have to become more "machine-readable". Like, for example, https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist ___ 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 submission questions
Thanks for the tips Andriy. To make it easier for future new contributors I took the time to update developers.texi with the information I got from this thread, and revamp the chapter about submitting patches. Feedback is most welcome. Best regards, Manolis On Sun, 5 Jul 2020 at 20:56, Andriy Gelman wrote: > On Sun, 05. Jul 20:34, Manolis Stamatogiannakis wrote: > > On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong > > wrote: > > > > > > > > You can use `git format-patch -v -n` to get patches like [PATCH > > > v2 1/20]. See git-format-patch documentation for more details. > > > > > > > > That didn't quite work. > > > > I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org > > HEAD~2..HEAD" and "git send-email -2". > > > > But the -v argument was dropped and the end results on patchwork were > again: > > [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for > > subtitles/ass filters. > > [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter > > options. > > > > git send-email -v2 HEAD~ # works for me > > git shows are draft of the email before sending. Check that the subject > line > contains the version number (or you can send the patch to yourself) > > For patchwork, you can create an account and set your old patches as > 'Superseded'. > I also periodically run a script that sets Superseded and Accepted labels. > > -- > Andriy > ___ > 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] Project orientation
> -Original Message- > From: ffmpeg-devel On Behalf Of > Jean-Baptiste Kempf > Sent: Sunday, July 5, 2020 11:48 PM > To: ffmpeg-devel > Subject: Re: [FFmpeg-devel] Project orientation > > On Sun, Jul 5, 2020, at 22:50, Michael Niedermayer wrote: > > > Having your > > > patch being not responded to (whether being forgotten, not found > > > interesting enough, or whatever other reason) was. > > > > At least for me the reason to not review a patch is often simply time. > > But i agree the amount of not reviewed patches is a problem. > > Yes. > > > How can we solve this ? > > With tools and organization. > > Today, any patchset that has several patches (like the 36 ones for VP9 of the > other day) or multiple revisions gets everyone a ton of emails. When you are > at the revision 3 or 7, you completely lost track, unless you know exactly > what to look for. Seeing the difference from the 2 revisions is very hard. So > either you spend a lot of time understanding and re-reading or you drop it. > So, either inefficiency for the reviewer, and therefore less reviews, or > forgotten patches. > > Today the patchwork has 64 pages of patches... > > Noone human can know all that is happening. > > So either you split the mailing list and patches per library (different for > libavcodec, format), with submaintainers for each library, basically, like the > Linux Kernel does. > Or you move to github/gitlab which gives you by default a tracking of the MR > that got merged or not, where you can see quickly the differences between > 2 revisions of a patchset and where you can use tags to mark the merge > requests, where the CI is automatic, so you don't have to check manually > that the code compiles or that FATE is not broken, etc... +1 for this. A significant part of code reviews are code-style violations. This is really not something where humans should need to spend time for when reviewing a patch. Neither should it be required that Michael responds manually each time by stating "breaks FATE". Also, this would allow review comments stay connected to the commented code parts forever. When in the future somebody wonders why something has been done in a certain way, it will be easy to find that discussion. softworkz ___ 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] lavfi/vf_vpp_qsv: fix the infinite loop while framerate lower than input
On Sun, Jul 5, 2020 at 5:56 PM Zhong Li wrote: > > > I can't see the benefit to use MSDK framerate conversion function. Is > > > it a good idea to drop it and use ffmpeg native fps filter instead? > > > > Reconsidering this, leaving the filter buggy doesn't seem to be > comfortable to me, > > hence IMHO it'll be better to have this bug fixed. > My suggestion would be just delete MSDK framerate conversion instead > of patch it. > Will send a patch if nobody against. > should be ok when comes with proper description, as comes with price of performance regards Max ___ 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] Project orientation
July 5, 2020 11:43 PM, "Manolis Stamatogiannakis" wrote: > On Sun, 5 Jul 2020 at 19:01, Kieran Kunhya wrote: > >> For new contributors git send-email is annoying. For people wanting to >> push, the .mbox format is annoying, Gmail doesn't support it any more. >> And you can't get new contributors to start using CLI based email >> clients or run their own mail server, that's not going to happen. > > As a fresh contributor, setting up git send-email was a hassle, but > not an insurmountable obstacle. > > Though I'd argue that git send-email is a bigger liability for regular > developers. A lot of developers seem to be using Gmail, which means they > need to have "less secure apps" enabled at the time they use git > send-email. So they are forced to choose between security and convenience. > I.e. having to temporarily enable "less secure apps" every time they > submit, or accept the risk of weakened security for their account. Although not exactly convenient, Gmail now has a feature called app passwords that allows you to create specialised passwords for things like e-mail clients. You however have to enable 2 factor authentication for this. https://support.google.com/accounts/answer/185833?hl=en >> A solution like Gitlab is the only way forward. It has worked well for >> dav1d, it can run regression tests on all platforms for all commits: >> https://code.videolan.org/videolan/dav1d >> >> Merges are done with one push of a button. Yes, the branch sprawl is >> not great but it's better than now. >> It has inline patch reviews which are nice. > > FWIW, I'd add that branch-based PRs as implemented with GitHub greatly > reduce the turnaround for receiving feedback and addressing the raised > issues. > All you have to do is force-push on your PR branch, and the PR is cleanly > updated. > This is in contrast with having to use git send-email, where the emails > containing the stale patches will continue to litter patchwork and your > mailboxes. > > Also, IIRC, when you update a branch, the regression tests for all pending > PRs on the branch are automatically rerun. > This should help address the conflicts in the pending PRs sooner rather > than later. > > Not sure what the status of GitLab is wrt to these features, but I'd expect > it to not be very far behind. > > Best regards, > Manolis Regards, Anamitra ___ 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 1/3] avcodec/mjpegdec: Limit bayer to single plane outputting format
On Sat, Jul 04, 2020 at 02:45:51PM +0200, Michael Niedermayer wrote: > This reduces the number of paths reachable with DNG and should > improve security > > Signed-off-by: Michael Niedermayer > --- > libavcodec/mjpegdec.c | 5 + > 1 file changed, 5 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle 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 2/3] avcodec/tiff: Check frame parameters before blit for DNG
On Sat, Jul 04, 2020 at 02:45:52PM +0200, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: > 23888/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-6021365974171648.fuzz > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/tiff.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued 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 1/2] libavcodec/jpeg2000dec: Enhance pix fmt selection
On Sun, Jul 05, 2020 at 12:33:08AM +0530, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > This patch assigns default pix format values when > a match does not take place. > --- > libavcodec/jpeg2000dec.c | 12 > 1 file changed, 12 insertions(+) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato 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 2/2] libavcodec/jpeg2000dec.c: Enable image offsets
On Sun, Jul 05, 2020 at 12:33:09AM +0530, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > This patch enables support for image offsets. > --- > libavcodec/jpeg2000dec.c | 4 > 1 file changed, 4 deletions(-) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf 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 v06 2/5] fbtile helperRoutines cpu based framebuffer detiling
On 04/07/2020 14:17, hanishkvc wrote:
Add helper routines which can be used to detile tiled framebuffer
layouts into a linear layout, using the cpu.
Currently it supports Legacy Intel Tile-X, Legacy Intel Tile-Y and
Newer Intel Tile-Yf tiled layouts.
Currently supported pixel format is 32bit RGB.
It also contains detile_generic logic, which can be easily configured
to support different kinds of tiling layouts, at the expense of some
processing speed, compared to developing a targeted detiling logic.
---
libavutil/Makefile | 2 +
libavutil/fbtile.c | 441 +
libavutil/fbtile.h | 228 +++
3 files changed, 671 insertions(+)
create mode 100644 libavutil/fbtile.c
create mode 100644 libavutil/fbtile.h
I do think this is a reasonable thing to include in FFmpeg, but please think
about what you actually want as a public API here.
Ideally you want to minimise the public API surface while providing something
clean and of general use.
So:
- It should probably operate on whole frames - copying pieces of frames or
single planes doesn't have any obvious use.
- The pixel format, width, height and pitches will all need to be specified, so
AVFrames which already include that information are probably the right
structure to use.
- You want to specify a tiling mode for the source frame somehow.
- For the untile case the destination is linear, but a plausible use-case is
the opposite so we could include tiling mode for the destination as well.
- The tiling modes will need to be some sort of enum, since they are all ad-hoc
setups for particular hardware vendors.
- We can't reuse existing values like those from libdrm because we'd like it to
work everywhere it can and there is no intrinsic dependence on libdrm, so it
needs to be a new enum.
- Names for the tiling modes should be valid everywhere, so if they are
platform-dependent (like Intel X/Y tiling) then the platform will need to be
included in the name.
- Linear should be in the tiling mode enum, so that you don't need special
cases elsewhere.
- All the dispatch between different implementations can happen internally, so
it doesn't need to be exposed.
- Not everything will actually be implemented, so it will need to be able to
return an error indicating that a particular case is not available.
Given that, I make the desired public API to be exactly one enum and one
function. It would look something like:
enum AVTilingMode {
AV_TILING_MODE_LINEAR = 0,
AV_TILING_MODE_INTEL_GEN7_X,
AV_TILING_MODE_INTEL_GEN7_Y,
AV_TILING_MODE_INTEL_GEN9_X,
AV_TILING_MODE_INTEL_GEN9_Y,
AV_TILING_MODE_INTEL_YF,
};
int av_frame_copy_with_tiling(const AVFrame *dst, enum AVTilingMode dst_tiling,
const AVFrame *src, enum AVTilingMode src_tiling);
Some other thoughts:
- Functions should all be static unless you are intentionally exposing them as
public API.
- Libraries must not include mutable globals.
- Note that av_log(NULL, ...) should never be called from a library unless you
really are in a global context. I think you probably just don't want to
include any user-facing logging at all.
- Look at the coding style guide, especially around naming and operator spacing.
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 1/7] avcodec/cbs: Check for overflow when reading
On 09/12/2019 22:25, Andreas Rheinhardt wrote:
While CBS itself uses size_t for sizes, it relies on other APIs that use
int for their sizes; in particular, AVBuffer uses int for their size
parameters and so does GetBitContext with their number of bits. While
CBS aims to be a safe API, the checks it employed were not sufficient to
prevent overflows: E.g. if the size of a unit was > UINT_MAX / 8, 8 *
said size may be truncated to a positive integer before being passed to
init_get_bits() in which case its return value would not indicate an
error.
These checks have been improved to really capture these kinds of errors;
furthermore, although the sizes are still size_t, they are now de-facto
restricted to 0..INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE.
Signed-off-by: Andreas Rheinhardt
---
The check in cbs_insert_unit() can currently not be triggered, because
av_malloc_array makes sure that it doesn't allocate more than INT_MAX;
so the allocation will fail long before we get even close to INT_MAX.
av1 and H.26x have not been changed, because their split functions
already check the size (in case of H.264 and H.265 this happens in
ff_h2645_packet_split()).
It would btw be possible to open the GetBitContext generically. The
read_unit function would then get the already opened GetBitContext
as a parameter.
libavcodec/cbs.c | 6 ++
libavcodec/cbs_jpeg.c | 2 +-
libavcodec/cbs_mpeg2.c | 2 +-
libavcodec/cbs_vp9.c | 2 +-
4 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 0badb192d9..805049404b 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -206,6 +206,9 @@ static int cbs_fill_fragment_data(CodedBitstreamContext
*ctx,
{
av_assert0(!frag->data && !frag->data_ref);
+if (size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
+return AVERROR(ERANGE);
For this and the following patch I wonder if it would be nicer to pick a sensible upper bounds for fragments (something like 2^30B total in at most 2^20 units?), name them (CBS_MAX_DATA_SIZE,
CBS_MAX_UNITS?) and then use those in all checks?
No real case should get anywhere near these bounds, and indeed anything which
even gets close was likely crafted by a malicious adversary specifically to do
so.
+
frag->data_ref =
av_buffer_alloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
if (!frag->data_ref)
@@ -693,6 +696,9 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx,
memmove(units + position + 1, units + position,
(frag->nb_units - position) * sizeof(*units));
} else {
+if (frag->nb_units == INT_MAX)
+return AVERROR(ERANGE);
+
units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
if (!units)
return AVERROR(ENOMEM);
diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
index b189cbd9b7..2bb6c8d18c 100644
--- a/libavcodec/cbs_jpeg.c
+++ b/libavcodec/cbs_jpeg.c
@@ -246,7 +246,7 @@ static int cbs_jpeg_read_unit(CodedBitstreamContext *ctx,
GetBitContext gbc;
int err;
-err = init_get_bits(&gbc, unit->data, 8 * unit->data_size);
+err = init_get_bits8(&gbc, unit->data, unit->data_size);
if (err < 0)
return err;
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 13d871cc89..255f033734 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -227,7 +227,7 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext *ctx,
GetBitContext gbc;
int err;
-err = init_get_bits(&gbc, unit->data, 8 * unit->data_size);
+err = init_get_bits8(&gbc, unit->data, unit->data_size);
if (err < 0)
return err;
diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
index 42e4dcf5ac..f6cfaa3b36 100644
--- a/libavcodec/cbs_vp9.c
+++ b/libavcodec/cbs_vp9.c
@@ -488,7 +488,7 @@ static int cbs_vp9_read_unit(CodedBitstreamContext *ctx,
GetBitContext gbc;
int err, pos;
-err = init_get_bits(&gbc, unit->data, 8 * unit->data_size);
+err = init_get_bits8(&gbc, unit->data, unit->data_size);
if (err < 0)
return err;
There are more of these init_get_bits(..., 8 * something) hanging around.
Perhaps change all of them as one patch, even if there isn't any danger of
overflow?
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] Project orientation
On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: > ... A significant part of code reviews are code-style violations. This is > really not something where humans should need to spend time for when > reviewing a patch. you are correct but that is also the easy part of reviews. Its not what makes reviews time consuming. Rather while reviewing for technical stuff one notices maybe a formating issue > Neither should it be required that Michael responds manually each time > by stating "breaks FATE". We already have fully automatic build and fate tests for submitted patches its done by patchwork which should send a private mail to the submitter of a patch affected. Still not every environment is the same, and it passing on the patchwork box doesnt mean it builds on my mingw or mips environments for example. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "I am not trying to be anyone's saviour, I'm trying to think about the future and not be sad" - Elon Musk signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org 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] Project orientation
On 06.07.2020 01:18, Michael Niedermayer wrote: On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: ... A significant part of code reviews are code-style violations. This is really not something where humans should need to spend time for when reviewing a patch. you are correct but that is also the easy part of reviews. Its not what makes reviews time consuming. Rather while reviewing for technical stuff one notices maybe a formating issue Neither should it be required that Michael responds manually each time by stating "breaks FATE". We already have fully automatic build and fate tests for submitted patches its done by patchwork which should send a private mail to the submitter of a patch affected. Still not every environment is the same, and it passing on the patchwork box doesnt mean it builds on my mingw or mips environments for example. It also very frequently fails to properly handle large and complex series, with some of the patches getting new versions. Also can't properly handle testing patches for stuff it cannot build, because it's for a different arch or needs a very specific library. But that would equally affect any kind of Gitlab/Github Workflow we could set up. ___ 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".
