[PATCH] SSA/ASS subtitles support
Hi Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska. The patch also adds attachment support to matroska demuxer, as it was needed to extract fonts that usually come with subtitles. Usage is -vf ass for mkv subs and -vf ass=<filename> for external. I tried to change as little as possible in the surrounding code, but still had to add some bad looking lines to mplayer.c and mencoder.c to pass current time and subtitle track to the filter. If there is better way to do it, i'd be glad to hear about it :)
Hello, On Friday 31 March 2006 23:09, Evgeniy Stepanov wrote:
Hi
Here is an attempt to add ASS rendering capability to mplayer. It is
Huhuhuhuhu. Built-in porn. Nice. :) (Sorry, couldn't resist as usual.)
designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
The patch also adds attachment support to matroska demuxer, as it was needed to extract fonts that usually come with subtitles.
Usage is -vf ass for mkv subs and -vf ass=<filename> for external.
I tried to change as little as possible in the surrounding code, but still had to add some bad looking lines to mplayer.c and mencoder.c to pass current time and subtitle track to the filter. If there is better way to do it, i'd be glad to hear about it :)
Rob
On Mar 31, 2006, at 5:09 PM, Evgeniy Stepanov wrote:
Hi
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
Some things I notice on OS X: * This uses strndup(), which doesn't exist outside glibc. * You left a hardcoded freetype path in libmpcodecs/Makefile. * Is it really necessary to memcpy every frame in the filter? I'm working on removing memcpy from vo_macosx, and I don't want more put in the filter chain. * Using fontconfig on OS X will be really bad for people who don't have X11 installed, since it will have to rescan the system font directories every time it starts. There's not really a good solution here, though, unless someone wants to write a fontconfig-ATSUI shim. Also, the kerning looks weird on some lines and the letters are all very thin compared to Windows' VSFilter or any other player's current subtitle handling, but I haven't investigated this very hard.
On Fri, Mar 31, 2006 at 11:23:26PM -0500, Alexander Strange wrote:
On Mar 31, 2006, at 5:09 PM, Evgeniy Stepanov wrote:
Hi
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
Some things I notice on OS X:
* This uses strndup(), which doesn't exist outside glibc.
This is very bad. You will get a lot of cola from me for using glibc-isms. :) Glad it was caught tho.
* Is it really necessary to memcpy every frame in the filter? I'm
It should be possible to render the subs along with copying the picture onto the output buffer, without any extra intermediate copies. But there's no way to avoid copies altogether. :( You're not allowed to modify the input buffer. Rich
On Saturday 01 April 2006 08:23, you wrote:
Some things I notice on OS X:
* This uses strndup(), which doesn't exist outside glibc. * You left a hardcoded freetype path in libmpcodecs/Makefile.
Fixed.
* Is it really necessary to memcpy every frame in the filter? I'm working on removing memcpy from vo_macosx, and I don't want more put in the filter chain.
Looks like it is unavoidable. But it is possible to not copy frames without subtitles, patch attached.
* Using fontconfig on OS X will be really bad for people who don't have X11 installed, since it will have to rescan the system font directories every time it starts. There's not really a good solution here, though, unless someone wants to write a fontconfig-ATSUI shim.
An option can be added to look only in ~/.mplayer/fonts directory, where all embedded fonts go. It is enough in 95% of cases.
Also, the kerning looks weird on some lines and the letters are all very thin compared to Windows' VSFilter or any other player's current subtitle handling, but I haven't investigated this very hard.
The border width is probally smaller then in VSFilter. You can try to increase it (either in subtitle script, or by adding a multiplier in the FT_Stroker_Set call) and see if it becomes better. There is a problem though. With some combinations of font size and border width there appears a strange gap in some glyph outlines. Maybe a bug in freetype ? The kerning is a little incorrect because I'm using integer pixel offsets when placing glyphs. Subpixel precision leads to poor bitmap cache performance, because almost all the glyphs become different. Still, 1/4 or 1/8 pixel precision might be acceptable. I'll try it some day.
On Apr 1, 2006, at 1:59 PM, Evgeniy Stepanov wrote:
The border width is probally smaller then in VSFilter. You can try to increase it (either in subtitle script, or by adding a multiplier in the FT_Stroker_Set call) and see if it becomes better. There is a problem though. With some combinations of font size and border width there appears a strange gap in some glyph outlines. Maybe a bug in freetype ?
The current patch actually ignores the border width in the SSA and sets it to 1. If I change that, it looks better at large sizes but still looks funny at small ones. I suspect this is partially because VSFilter uses a better method for subsampling chroma in 4:2:0 (but macosx displays in 4:2:2 anyway, so if the renderer supports that it would improve). The rest might be freetype's imperfect hinting; I think you have to actually enable the auto-hinter manually? Also, why does it scale the font size by .6?
The kerning is a little incorrect because I'm using integer pixel offsets when placing glyphs. Subpixel precision leads to poor bitmap cache performance, because almost all the glyphs become different. Still, 1/4 or 1/8 pixel precision might be acceptable. I'll try it some day.
I disabled the cache and only had a noticeable performance loss in one movie, so having a correct-but-slower mode would be fine with me. It would also be useful in mencoder. Some other things: * I went and rebuilt mplayer with fontconfig on, and ass_fontconfig.c never matches any fonts, all of which fc-match worked with. I copied and pasted the fontconfig matching code from font_load_ft and it worked (if you add in an FC_INDEX query, since the old one ignored it). I can look into this further if you want. * Japanese subtitles clip out the side because it doesn't automatically linebreak them, since they have no whitespace. I think fribidi might handle this sort of thing, but I'm not sure about that. * This file asserts out: http://raspberryheaven.net/~adtrw/anime/ Makoto%20Shinkai%20-%20Other%20Worlds.mkv ass_render.c:1200: failed assertion `frame_context.dmpi->stride[1] == frame_context.dmpi->chroma_width' (The mistimed subtitles are my fault, not yours.)
On Monday 03 April 2006 07:24, Alexander Strange wrote:
The current patch actually ignores the border width in the SSA and sets it to 1. If I change that, it looks better at large sizes but still looks funny at small ones. I suspect this is partially because VSFilter uses a better method for subsampling chroma in 4:2:0 (but macosx displays in 4:2:2 anyway, so if the renderer supports that it would improve). The rest might be freetype's imperfect hinting; I think you have to actually enable the auto-hinter manually?
Can you try enabling the auto-hinter then ? With increased border width, it looks almost perfect to me already. Must be FT_LOAD_FORCE_AUTOHINT option. {\bordN} override value is used, style value isn't. I'll fix it.
Also, why does it scale the font size by .6?
The text is really huge without it :) With .6 it is about VSFilter's size. Maybe the resolution values are wrong ? It could also be the reason for weird hinting.
I disabled the cache and only had a noticeable performance loss in one movie, so having a correct-but-slower mode would be fine with me. It would also be useful in mencoder.
Try ftp://toro.imec.msu.ru/pub/users/eugeni/output.mkv with and without cache.
Some other things: * I went and rebuilt mplayer with fontconfig on, and ass_fontconfig.c never matches any fonts, all of which fc-match worked with. I copied and pasted the fontconfig matching code from font_load_ft and it worked (if you add in an FC_INDEX query, since the old one ignored it). I can look into this further if you want.
Sure, that would be great.
* Japanese subtitles clip out the side because it doesn't automatically linebreak them, since they have no whitespace. I think fribidi might handle this sort of thing, but I'm not sure about that. * This file asserts out: http://raspberryheaven.net/~adtrw/anime/ Makoto%20Shinkai%20-%20Other%20Worlds.mkv ass_render.c:1200: failed assertion `frame_context.dmpi->stride[1] == frame_context.dmpi->chroma_width' (The mistimed subtitles are my fault, not yours.)
On Sat, 1 Apr 2006 02:09:32 +0400 "Evgeniy Stepanov" <eugeni.stepanov@gmail.com> wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
The patch also adds attachment support to matroska demuxer, as it was needed to extract fonts that usually come with subtitles.
Usage is -vf ass for mkv subs and -vf ass=<filename> for external.
I just had a short look at it and a few things came to my mind: 1) would it be possible to make this more general to handle other subtitles too? Then we could get rid of the current subtitle mess. 2) The filter can just handle YV12, which is bad, it should at least be able to handle the most comon YUV formats. 3) i'd put the subtitle code into its own directory than poluting libmpcodecs which already has too many files. 4) Had you have a look at vf_expand.c ? it does partially the same as you do. 5) would you be willing to take over the maintenance of the subtitle code?
I tried to change as little as possible in the surrounding code, but still had to add some bad looking lines to mplayer.c and mencoder.c to pass current time and subtitle track to the filter. If there is better way to do it, i'd be glad to hear about it :)
Bug Rich, either here on the ML or on IRC. He can explain you all the quirks of the vf system and how time is passed trough mplayer. Attila Kinali -- 心をこめて聞け心をこめて話せ
On Saturday 01 April 2006 17:11, Attila Kinali wrote:
I just had a short look at it and a few things came to my mind: 1) would it be possible to make this more general to handle other subtitles too? Then we could get rid of the current subtitle mess.
Surely it's possible. All the subtitle sources (subreader + demuxers) have to be modified to write subtitle events to new structures instead of subdata. Its format is expressive enough to support anything (vobsub excepted).
2) The filter can just handle YV12, which is bad, it should at least be able to handle the most comon YUV formats.
Agreed. I'll do it in some time.
3) i'd put the subtitle code into its own directory than poluting libmpcodecs which already has too many files.
Already in the latest patch.
4) Had you have a look at vf_expand.c ? it does partially the same as you do.
It does not have to draw multiple overlapping transparent images. I've given it much thought and looks like it is impossible in YV12. So i'm converting parts of the image containing subtitles to 4:4:4 and back.
5) would you be willing to take over the maintenance of the subtitle code?
I would not be able to give it much time, plus reading the current subtitle code gives me headaches ;) I can look into general subtitle support in the filter though.
On Sat, Apr 01, 2006 at 03:11:46PM +0200, Attila Kinali wrote:
On Sat, 1 Apr 2006 02:09:32 +0400 "Evgeniy Stepanov" <eugeni.stepanov@gmail.com> wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
The patch also adds attachment support to matroska demuxer, as it was needed to extract fonts that usually come with subtitles.
Usage is -vf ass for mkv subs and -vf ass=<filename> for external.
I just had a short look at it and a few things came to my mind: 1) would it be possible to make this more general to handle other subtitles too? Then we could get rid of the current subtitle mess.
It will never be able to compete with the performance of the current subtitle rendering, which cheats by taking place inside the vo driver.
2) The filter can just handle YV12, which is bad, it should at least be able to handle the most comon YUV formats.
I don't see how that matters too much. No one uses any format except 4:2:0 planar, especially with subtitles. Rich
On Sunday 02 April 2006 05:26, Rich Felker wrote:
2) The filter can just handle YV12, which is bad, it should at least be able to handle the most comon YUV formats.
I don't see how that matters too much. No one uses any format except 4:2:0 planar, especially with subtitles.
-vo png, for example, wants something RGB-like. Sure, it's usually not performance-critical, and -vf scale helps. But still an inconvenience. Also, it should work considerably faster with RGB and 4:4:4 planar formats. The latest patch contained a bug, corrected version attached.
On Sunday, 02 April 2006 at 03:49, Evgeniy Stepanov wrote:
On Sunday 02 April 2006 05:26, Rich Felker wrote:
2) The filter can just handle YV12, which is bad, it should at least be able to handle the most comon YUV formats.
I don't see how that matters too much. No one uses any format except 4:2:0 planar, especially with subtitles.
-vo png, for example, wants something RGB-like. Sure, it's usually not performance-critical, and -vf scale helps. But still an inconvenience. Also, it should work considerably faster with RGB and 4:4:4 planar formats.
The latest patch contained a bug, corrected version attached.
All I get is a segfault, with today's CVS: $ gdb ~/build/pkg/BUILD/MPlayer-20060402/mplayer GNU gdb Red Hat Linux (6.3.0.0-1.84rh) Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "i386-redhat-linux-gnu"...Using host libthread_db library "/lib/libthread_db.so.1". (gdb) run -vf ass=GITS_SAC_\[1x01\].\[iNOS\].ssa GITS_SAC_\[1x01\].\[iNOS\].avi -v Starting program: /mnt/builder/dominik/pkg/BUILD/MPlayer-20060402/mplayer -vf ass=GITS_SAC_\[1x01\].\[iNOS\].ssa GITS_SAC_\[1x01\].\[iNOS\].avi -v Reading symbols from shared object read from target memory...done. Loaded system supplied DSO at 0xaa0000 [Thread debugging using libthread_db enabled] [New Thread -1208424768 (LWP 14390)] MPlayer dev-Fedora-GS-CVS-060402-21:19-4.0.2 (C) 2000-2006 MPlayer Team CPU: Advanced Micro Devices Athlon Thunderbird (Family: 6, Stepping: 2) CPUflags: MMX: 1 MMX2: 1 3DNow: 1 3DNow2: 1 SSE: 0 SSE2: 0 Compiled for x86 CPU with extensions: MMX MMX2 3DNow 3DNowEx get_path('codecs.conf') -> '/home/dominik/.mplayer/codecs.conf' Reading /home/dominik/.mplayer/codecs.conf: Can't open '/home/dominik/.mplayer/codecs.conf': No such file or directory Reading /etc/mplayer/codecs.conf: Can't open '/etc/mplayer/codecs.conf': No such file or directory Using built-in default codecs.conf. CommandLine: '-vf' 'ass=GITS_SAC_[1x01].[iNOS].ssa' 'GITS_SAC_[1x01].[iNOS].avi' '-v' init_freetype Using MMX (with tiny bit MMX2) Optimized OnScreenDisplay Using Linux hardware RTC timing (1024Hz). get_path('input.conf') -> '/home/dominik/.mplayer/input.conf' Parsing input config file /home/dominik/.mplayer/input.conf Input config file /home/dominik/.mplayer/input.conf parsed: 62 binds get_path('menu.conf') -> '/home/dominik/.mplayer/menu.conf' [MENU] Can't open menu config file: /home/dominik/.mplayer/menu.conf Menu inited: /etc/mplayer/menu.conf get_path('GITS_SAC_[1x01].[iNOS].avi.conf') -> '/home/dominik/.mplayer/GITS_SAC_[1x01].[iNOS].avi.conf' Playing GITS_SAC_[1x01].[iNOS].avi. get_path('sub/') -> '/home/dominik/.mplayer/sub/' [file] File size is 367048704 bytes STREAM: [file] GITS_SAC_[1x01].[iNOS].avi STREAM: Description: File STREAM: Author: Albeu STREAM: Comment: based on the code from ??? (probably Arpi) AVI file format detected. list_end=0x22CA ======= AVI Header ======= us/frame: 33367 (fps=29.970) max bytes/sec: 0 padding: 0 MainAVIHeader.dwFlags: (272) HAS_INDEX IS_INTERLEAVED frames total: 47130 initial: 0 streams: 2 Suggested BufferSize: 0 Size: 800 x 448 ========================== list_end=0x1138 ==> Found video stream: 0 ====== STREAM Header ===== Type: vids FCC: xvid (64697678) Flags: 0 Priority: 0 Language: 0 InitialFrames: 0 Rate: 29970/1000 = 29.970 Start: 0 Len: 47130 Suggested BufferSize: 97827 Quality 10000 Sample size: 0 ========================== Found 'bih', 108 bytes of 40 ======= VIDEO Format ====== biSize 40 biWidth 800 biHeight 448 biPlanes 1 biBitCount 16 biCompression 1145656920='XVID' biSizeImage 2150400 =========================== Regenerating keyframe table for MPEG4 video list_end=0x21BE ==> Found audio stream: 1 ====== STREAM Header ===== Type: auds FCC: (0) Flags: 0 Priority: 0 Language: 0 InitialFrames: 1 Rate: 56000/1 = 56000.000 Start: 0 Len: 88060893 Suggested BufferSize: 28000 Quality -1 Sample size: 1 ========================== Found 'wf', 18 bytes of 18 ======= WAVE Format ======= Format Tag: 8192 (0x2000) Channels: 5 Samplerate: 48000 avg byte/sec: 56000 Block align: 1 bits/sample: 0 cbSize: 18 Unknown extra header dump: [0] [0] [74] [6c] [65] [3d] [22] [56] [69] [64] [65] [6f] [22] [0] [28] [0] [0] [0] ========================================================================== list_end=0x22CA AVI: dmlh found (size=248) (total_frames=47130) list_end=0x239E hdr=Software size=15 Software : Nandub v1.0rc2 hdr=Name size=49 Name : Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS hdr=Subject size=49 Subject : Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS hdr=Artist size=13 Artist : iNOS-kang29k hdr=Copyright size=20 Copyright : It's none of us !!! hdr=Comments size=1 Comments : list_end=0x15C9AFA6 Found movie at 0x280C - 0x15C9AFA6 Reading INDEX block, 94244 chunks for 47130 frames (fpos=365539246) AVI index offset: 0x2808 (movi=0x280C idx0=0x4 idx1=0x6D6C) Auto-selected AVI audio ID = 1 Auto-selected AVI video ID = 0 AVI: Searching for audio stream (id:1) AVI video size=276664482 (47130) audio size=88060893 (88060893) VIDEO: [XVID] 800x448 16bpp 29.970 fps 1407.4 kbps (171.8 kbyte/s) [V] filefmt:3 fourcc:0x44495658 size:800x448 fps:29.97 ftime:=0.0334 Clip info: Software: Nandub v1.0rc2 Name: Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS Subject: Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS Artist: iNOS-kang29k Copyright: It's none of us !!! Comments: get_path('sub/') -> '/home/dominik/.mplayer/sub/' [file] File size is 42979 bytes STREAM: [file] ./GITS_SAC_[1x01].[iNOS].smi STREAM: Description: File STREAM: Author: Albeu STREAM: Comment: based on the code from ??? (probably Arpi) SUB: Detected subtitle file format: sami SUB: opened iconv descriptor. SUB: closed iconv descriptor. SUB: Read 285 subtitles. SUB: added subtitle file (1): ./GITS_SAC_[1x01].[iNOS].smi [file] File size is 36223 bytes STREAM: [file] ./GITS_SAC_[1x01].[iNOS].ssa STREAM: Description: File STREAM: Author: Albeu STREAM: Comment: based on the code from ??? (probably Arpi) SUB: Detected subtitle file format: ssa SUB: opened iconv descriptor. SUB: closed iconv descriptor. SUB: Read 305 subtitles. SUB: Adjusted 83 subtitle(s). SUB: added subtitle file (2): ./GITS_SAC_[1x01].[iNOS].ssa ========================================================================== Opening audio decoder: [liba52] AC3 decoding with liba52 dec_audio: Allocating 3840 bytes for input buffer. dec_audio: Allocating 6144 + 65536 = 71680 bytes for output buffer. Using 3DNowEx optimized IMDCT transform AC3: 5.1 (3f+2r+lfe) 48000 Hz 448.0 kbit/s A52 flags before a52_frame: 0x2A A52 flags after a52_frame: 0xA Using MMX optimized resampler AUDIO: 48000 Hz, 2 ch, s16le, 448.0 kbit/29.17% (ratio: 56000->192000) Selected audio codec: [a52] afm: liba52 (AC3-liba52) ========================================================================== X11 opening display: :0.0 vo: X11 color mask: FFFFFF (R:FF0000 G:FF00 B:FF) vo: X11 running at 1152x864 with depth 24 and 32 bpp (":0.0" => local display) [x11] Detected wm supports NetWM. [x11] Detected wm supports FULLSCREEN state. [x11] Detected wm supports ABOVE state. [x11] Detected wm supports BELOW state. [x11] Current fstype setting honours FULLSCREEN ABOVE BELOW X atoms xscreensaver_disable: xscreensaver wid=10485761. Detaching after fork from child process 14393. [xv common] Drawing colorkey manually. [xv common] Using colorkey from Xv (0x0101fe). [xv common] Maximum source image dimensions: 2046x2046 Opening video filter: [ass=GITS_SAC_[1x01].[iNOS].ssa] get_path('subfont.ttf') -> '/home/dominik/.mplayer/subfont.ttf' get_path('fonts') -> '/home/dominik/.mplayer/fonts' file size: 36223 [49] Style: Default,Arial,15,16777215,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,0 [49] Style: Russian,Arial,15,14772545,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,204 [49] Style: Song,Arial,15,12234746,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,0 [49] Style: *Default,Arial,15,16777215,65535,65535,0,-1,0,1,3,0,2,30,30,30,0,0 ========================================================================== Opening video decoder: [ffmpeg] FFmpeg's libavcodec codec family INFO: libavcodec init OK! Selected video codec: [ffodivx] vfm: ffmpeg (FFmpeg MPEG-4) ========================================================================== Building audio filter chain for 48000Hz/2ch/s16le -> 0Hz/0ch/??... [libaf] Adding filter volnorm alsa-init: requested format: 48000 Hz, 2 channels, 9 alsa-init: using ALSA 1.0.10 alsa-init: setup for 1/2 channel(s) alsa-init: using device hw:0,0 alsa-init: pcm opend in blocking mode alsa-init: chunksize set to 1024 alsa-init: fragcount=16 alsa-init: got buffersize=65536 alsa-init: got period size 1024 alsa: 44100 Hz/2 channels/4 bpf/65536 bytes buffer/Signed 16 bit Little Endian AO: [alsa] 44100Hz 2ch s16le (2 bytes per sample) AO: Description: ALSA-0.9.x-1.x audio output AO: Author: Alex Beregszaszi, Zsolt Barat <joy@streamminister.de> AO: Comment: under developement Building audio filter chain for 48000Hz/2ch/s16le -> 44100Hz/2ch/s16le... [libaf] Adding filter lavcresample Starting playback... [libaf] Reallocating memory in module lavcresample, old len = 0, new len = 60212 [ffmpeg] aspect_ratio: 1.785714 VDec: vo config request - 800 x 448 (preferred colorspace: Planar YV12) Trying filter chain: ass menu vo VDec: using Planar YV12 as output csp (no 0) Movie-Aspect is 1.79:1 - prescaling to correct movie aspect. VO Config (800x448->800x448,flags=0,'MPlayer',0x32315659) REQ: flags=0x437 req=0x400 Unicode font: 656 glyphs. REQ: flags=0x437 req=0x0 VO: [xv] 800x448 => 800x448 Planar YV12 VO: Description: X11/Xv VO: Author: Gerd Knorr <kraxel@goldbach.in-berlin.de> and others Xvideo image format: 0x32595559 (YUY2) packed Xvideo image format: 0x32315659 (YV12) planar Xvideo image format: 0x59565955 (UYVY) packed Xvideo image format: 0x30323449 (I420) planar using Xvideo port 61 for hw scaling [xv] dx: 0 dy: 0 dw: 800 dh: 448 *** [ass] Allocating mp_image_t, 800x448x12bpp YUV planar, 537600 bytes *** [vo] Exporting mp_image_t, 800x448x12bpp YUV planar, 537600 bytes [xv] dx: 0 dy: 0 dw: 800 dh: 448 XXX initial v_pts=0.000 a_pos=28000 (0.500) 1 ??% ??% ??,?% 0 0 *** [ass] Allocating mp_image_t, 800x448x12bpp YUV planar, 537600 bytes *** [vo] Allocating mp_image_t, 800x448x12bpp YUV planar, 537600 bytes *** [menu] Direct Rendering mp_image_t, 800x448x12bpp YUV planar, 537600 bytes Neither PlayResX nor PlayResY defined. Using screen dimensions. fontconfig_select: using default font: (Arial) -> /home/dominik/.mplayer/subfont.ttf, 0 Error opening font Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1208424768 (LWP 14390)] 0x081ffe44 in ass_render_event (event=0x8d17100) at ass_render.c:740 740 error = FT_Stroker_New( render_context.face->memory, &render_context.stroker ); (gdb) bt #0 0x081ffe44 in ass_render_event (event=0x8d17100) at ass_render.c:740 #1 0x08142b1d in put_image (vf=0x8cc5c48, mpi=0x8e671b0, pts=-9.2233720368547758e+18) at vf_ass.c:131 #2 0x080ffabd in decode_video (sh_video=0x8ca4ed8, start=0x8ecf660 "", in_size=16946, drop_frame=0, pts=-9.2233720368547758e+18) at dec_video.c:339 #3 0x080a3373 in main (argc=5, argv=0xbfba7514) at mplayer.c:3562 (gdb) disp render_context.stroker 1: render_context.stroker = 0x0 SSA file attached. Regards, R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
Looks like you don't have an Arial font, and opening ~/.mplayer/subfont.ttf (which is used in such cases) failed. Attached version won't segfault.
On Monday, 03 April 2006 at 17:33, Evgeniy Stepanov wrote:
Looks like you don't have an Arial font, and opening ~/.mplayer/subfont.ttf
No, I don't.
(which is used in such cases) failed.
MPlayer supports font selection through fontconfig and I have the following in my ~/.mplayer/config: fontconfig=yes font="Nimbus Sans L"
Attached version won't segfault.
I'll test tonight. R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Monday, 03 April 2006 at 17:33, Evgeniy Stepanov wrote:
Looks like you don't have an Arial font, and opening ~/.mplayer/subfont.ttf (which is used in such cases) failed. Attached version won't segfault.
Still segfaults. $ gdb ~/build/pkg/BUILD/MPlayer-20060403/mplayer GNU gdb Red Hat Linux (6.3.0.0-1.84rh) Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "i386-redhat-linux-gnu"...Using host libthread_db library "/lib/libthread_db.so.1". (gdb) run -v -vf ass=GITS_SAC_\[1x01\].\[iNOS\].ssa GITS_SAC_\[1x01\].\[iNOS\].avi Starting program: /mnt/builder/dominik/pkg/BUILD/MPlayer-20060403/mplayer -v -vf ass=GITS_SAC_\[1x01\].\[iNOS\].ssa GITS_SAC_\[1x01\].\[iNOS\].avi Reading symbols from shared object read from target memory...done. Loaded system supplied DSO at 0x901000 [Thread debugging using libthread_db enabled] [New Thread -1208174912 (LWP 18098)] MPlayer dev-Fedora-GS-CVS-060403-21:41-4.0.2 (C) 2000-2006 MPlayer Team CPU: Advanced Micro Devices Athlon Thunderbird (Family: 6, Stepping: 2) CPUflags: MMX: 1 MMX2: 1 3DNow: 1 3DNow2: 1 SSE: 0 SSE2: 0 Compiled for x86 CPU with extensions: MMX MMX2 3DNow 3DNowEx get_path('codecs.conf') -> '/home/dominik/.mplayer/codecs.conf' Reading /home/dominik/.mplayer/codecs.conf: Can't open '/home/dominik/.mplayer/codecs.conf': No such file or directory Reading /etc/mplayer/codecs.conf: Can't open '/etc/mplayer/codecs.conf': No such file or directory Using built-in default codecs.conf. CommandLine: '-v' '-vf' 'ass=GITS_SAC_[1x01].[iNOS].ssa' 'GITS_SAC_[1x01].[iNOS].avi' init_freetype Using MMX (with tiny bit MMX2) Optimized OnScreenDisplay Using Linux hardware RTC timing (1024Hz). get_path('input.conf') -> '/home/dominik/.mplayer/input.conf' Parsing input config file /home/dominik/.mplayer/input.conf Input config file /home/dominik/.mplayer/input.conf parsed: 62 binds get_path('menu.conf') -> '/home/dominik/.mplayer/menu.conf' [MENU] Can't open menu config file: /home/dominik/.mplayer/menu.conf Menu inited: /etc/mplayer/menu.conf get_path('GITS_SAC_[1x01].[iNOS].avi.conf') -> '/home/dominik/.mplayer/GITS_SAC_[1x01].[iNOS].avi.conf' Playing GITS_SAC_[1x01].[iNOS].avi. get_path('sub/') -> '/home/dominik/.mplayer/sub/' [file] File size is 367048704 bytes STREAM: [file] GITS_SAC_[1x01].[iNOS].avi STREAM: Description: File STREAM: Author: Albeu STREAM: Comment: based on the code from ??? (probably Arpi) AVI file format detected. list_end=0x22CA ======= AVI Header ======= us/frame: 33367 (fps=29.970) max bytes/sec: 0 padding: 0 MainAVIHeader.dwFlags: (272) HAS_INDEX IS_INTERLEAVED frames total: 47130 initial: 0 streams: 2 Suggested BufferSize: 0 Size: 800 x 448 ========================== list_end=0x1138 ==> Found video stream: 0 ====== STREAM Header ===== Type: vids FCC: xvid (64697678) Flags: 0 Priority: 0 Language: 0 InitialFrames: 0 Rate: 29970/1000 = 29.970 Start: 0 Len: 47130 Suggested BufferSize: 97827 Quality 10000 Sample size: 0 ========================== Found 'bih', 108 bytes of 40 ======= VIDEO Format ====== biSize 40 biWidth 800 biHeight 448 biPlanes 1 biBitCount 16 biCompression 1145656920='XVID' biSizeImage 2150400 =========================== Regenerating keyframe table for MPEG4 video list_end=0x21BE ==> Found audio stream: 1 ====== STREAM Header ===== Type: auds FCC: (0) Flags: 0 Priority: 0 Language: 0 InitialFrames: 1 Rate: 56000/1 = 56000.000 Start: 0 Len: 88060893 Suggested BufferSize: 28000 Quality -1 Sample size: 1 ========================== Found 'wf', 18 bytes of 18 ======= WAVE Format ======= Format Tag: 8192 (0x2000) Channels: 5 Samplerate: 48000 avg byte/sec: 56000 Block align: 1 bits/sample: 0 cbSize: 18 Unknown extra header dump: [0] [0] [74] [6c] [65] [3d] [22] [56] [69] [64] [65] [6f] [22] [0] [28] [0] [0] [0] ========================================================================== list_end=0x22CA AVI: dmlh found (size=248) (total_frames=47130) list_end=0x239E hdr=Software size=15 Software : Nandub v1.0rc2 hdr=Name size=49 Name : Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS hdr=Subject size=49 Subject : Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS hdr=Artist size=13 Artist : iNOS-kang29k hdr=Copyright size=20 Copyright : It's none of us !!! hdr=Comments size=1 Comments : list_end=0x15C9AFA6 Found movie at 0x280C - 0x15C9AFA6 Reading INDEX block, 94244 chunks for 47130 frames (fpos=365539246) AVI index offset: 0x2808 (movi=0x280C idx0=0x4 idx1=0x6D6C) Auto-selected AVI audio ID = 1 Auto-selected AVI video ID = 0 AVI: Searching for audio stream (id:1) AVI video size=276664482 (47130) audio size=88060893 (88060893) VIDEO: [XVID] 800x448 16bpp 29.970 fps 1407.4 kbps (171.8 kbyte/s) [V] filefmt:3 fourcc:0x44495658 size:800x448 fps:29.97 ftime:=0.0334 Clip info: Software: Nandub v1.0rc2 Name: Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS Subject: Ghost.In.The.Shell-Stand.Alone.Complex.EP01_iNOS Artist: iNOS-kang29k Copyright: It's none of us !!! Comments: get_path('sub/') -> '/home/dominik/.mplayer/sub/' [file] File size is 42979 bytes STREAM: [file] ./GITS_SAC_[1x01].[iNOS].smi STREAM: Description: File STREAM: Author: Albeu STREAM: Comment: based on the code from ??? (probably Arpi) SUB: Detected subtitle file format: sami SUB: opened iconv descriptor. SUB: closed iconv descriptor. SUB: Read 285 subtitles. SUB: added subtitle file (1): ./GITS_SAC_[1x01].[iNOS].smi [file] File size is 36223 bytes STREAM: [file] ./GITS_SAC_[1x01].[iNOS].ssa STREAM: Description: File STREAM: Author: Albeu STREAM: Comment: based on the code from ??? (probably Arpi) SUB: Detected subtitle file format: ssa SUB: opened iconv descriptor. SUB: closed iconv descriptor. SUB: Read 305 subtitles. SUB: Adjusted 83 subtitle(s). SUB: added subtitle file (2): ./GITS_SAC_[1x01].[iNOS].ssa ========================================================================== Opening audio decoder: [liba52] AC3 decoding with liba52 dec_audio: Allocating 3840 bytes for input buffer. dec_audio: Allocating 6144 + 65536 = 71680 bytes for output buffer. Using 3DNowEx optimized IMDCT transform AC3: 5.1 (3f+2r+lfe) 48000 Hz 448.0 kbit/s A52 flags before a52_frame: 0x2A A52 flags after a52_frame: 0xA Using MMX optimized resampler AUDIO: 48000 Hz, 2 ch, s16le, 448.0 kbit/29.17% (ratio: 56000->192000) Selected audio codec: [a52] afm: liba52 (AC3-liba52) ========================================================================== X11 opening display: :0.0 vo: X11 color mask: FFFFFF (R:FF0000 G:FF00 B:FF) vo: X11 running at 1152x864 with depth 24 and 32 bpp (":0.0" => local display) [x11] Detected wm supports NetWM. [x11] Detected wm supports FULLSCREEN state. [x11] Detected wm supports ABOVE state. [x11] Detected wm supports BELOW state. [x11] Current fstype setting honours FULLSCREEN ABOVE BELOW X atoms Disabling DPMS DPMSDisable stat: 1 xscreensaver_disable: xscreensaver wid=10485761. Detaching after fork from child process 18101. [xv common] Drawing colorkey manually. [xv common] Using colorkey from Xv (0x0101fe). [xv common] Maximum source image dimensions: 2046x2046 Opening video filter: [ass=GITS_SAC_[1x01].[iNOS].ssa] get_path('subfont.ttf') -> '/home/dominik/.mplayer/subfont.ttf' get_path('fonts') -> '/home/dominik/.mplayer/fonts' file size: 36223 [49] Style: Default,Arial,15,16777215,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,0 [49] Style: Russian,Arial,15,14772545,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,204 [49] Style: Song,Arial,15,12234746,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,0 [49] Style: *Default,Arial,15,16777215,65535,65535,0,-1,0,1,3,0,2,30,30,30,0,0 ========================================================================== Opening video decoder: [ffmpeg] FFmpeg's libavcodec codec family INFO: libavcodec init OK! Selected video codec: [ffodivx] vfm: ffmpeg (FFmpeg MPEG-4) ========================================================================== Building audio filter chain for 48000Hz/2ch/s16le -> 0Hz/0ch/??... [libaf] Adding filter volnorm alsa-init: requested format: 48000 Hz, 2 channels, 9 alsa-init: using ALSA 1.0.10 alsa-init: setup for 1/2 channel(s) alsa-init: using device hw:0,0 alsa-init: pcm opend in blocking mode alsa-init: chunksize set to 1024 alsa-init: fragcount=16 alsa-init: got buffersize=65536 alsa-init: got period size 1024 alsa: 44100 Hz/2 channels/4 bpf/65536 bytes buffer/Signed 16 bit Little Endian dec_audio: Allocating 3840 bytes for input buffer. dec_audio: Allocating 6144 + 65536 = 71680 bytes for output buffer. Using 3DNowEx optimized IMDCT transform AC3: 5.1 (3f+2r+lfe) 48000 Hz 448.0 kbit/s A52 flags before a52_frame: 0x2A A52 flags after a52_frame: 0xA Using MMX optimized resampler AUDIO: 48000 Hz, 2 ch, s16le, 448.0 kbit/29.17% (ratio: 56000->192000) Selected audio codec: [a52] afm: liba52 (AC3-liba52) ========================================================================== X11 opening display: :0.0 vo: X11 color mask: FFFFFF (R:FF0000 G:FF00 B:FF) vo: X11 running at 1152x864 with depth 24 and 32 bpp (":0.0" => local display) [x11] Detected wm supports NetWM. [x11] Detected wm supports FULLSCREEN state. [x11] Detected wm supports ABOVE state. [x11] Detected wm supports BELOW state. [x11] Current fstype setting honours FULLSCREEN ABOVE BELOW X atoms Disabling DPMS DPMSDisable stat: 1 xscreensaver_disable: xscreensaver wid=10485761. Detaching after fork from child process 18101. [xv common] Drawing colorkey manually. [xv common] Using colorkey from Xv (0x0101fe). [xv common] Maximum source image dimensions: 2046x2046 Opening video filter: [ass=GITS_SAC_[1x01].[iNOS].ssa] get_path('subfont.ttf') -> '/home/dominik/.mplayer/subfont.ttf' get_path('fonts') -> '/home/dominik/.mplayer/fonts' file size: 36223 [49] Style: Default,Arial,15,16777215,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,0 [49] Style: Russian,Arial,15,14772545,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,204 [49] Style: Song,Arial,15,12234746,65535,65535,-2147483640,-1,0,1,3,0,2,30,30,30,0,0 [49] Style: *Default,Arial,15,16777215,65535,65535,0,-1,0,1,3,0,2,30,30,30,0,0 ========================================================================== Opening video decoder: [ffmpeg] FFmpeg's libavcodec codec family INFO: libavcodec init OK! Selected video codec: [ffodivx] vfm: ffmpeg (FFmpeg MPEG-4) ========================================================================== Building audio filter chain for 48000Hz/2ch/s16le -> 0Hz/0ch/??... [libaf] Adding filter volnorm alsa-init: requested format: 48000 Hz, 2 channels, 9 alsa-init: using ALSA 1.0.10 alsa-init: setup for 1/2 channel(s) alsa-init: using device hw:0,0 alsa-init: pcm opend in blocking mode alsa-init: chunksize set to 1024 alsa-init: fragcount=16 alsa-init: got buffersize=65536 alsa-init: got period size 1024 alsa: 44100 Hz/2 channels/4 bpf/65536 bytes buffer/Signed 16 bit Little Endian AO: [alsa] 44100Hz 2ch s16le (2 bytes per sample) AO: Description: ALSA-0.9.x-1.x audio output AO: Author: Alex Beregszaszi, Zsolt Barat <joy@streamminister.de> AO: Comment: under developement Building audio filter chain for 48000Hz/2ch/s16le -> 44100Hz/2ch/s16le... [libaf] Adding filter lavcresample Starting playback... [libaf] Reallocating memory in module lavcresample, old len = 0, new len = 60212 [ffmpeg] aspect_ratio: 1.785714 VDec: vo config request - 800 x 448 (preferred colorspace: Planar YV12) Trying filter chain: ass menu vo VDec: using Planar YV12 as output csp (no 0) Movie-Aspect is 1.79:1 - prescaling to correct movie aspect. VO Config (800x448->800x448,flags=0,'MPlayer',0x32315659) REQ: flags=0x437 req=0x400 Unicode font: 656 glyphs. REQ: flags=0x437 req=0x0 VO: [xv] 800x448 => 800x448 Planar YV12 VO: Description: X11/Xv VO: Author: Gerd Knorr <kraxel@goldbach.in-berlin.de> and others Xvideo image format: 0x32595559 (YUY2) packed Xvideo image format: 0x32315659 (YV12) planar Xvideo image format: 0x59565955 (UYVY) packed Xvideo image format: 0x30323449 (I420) planar using Xvideo port 61 for hw scaling [xv] dx: 0 dy: 0 dw: 800 dh: 448 *** [ass] Allocating mp_image_t, 800x448x12bpp YUV planar, 537600 bytes *** [vo] Exporting mp_image_t, 800x448x12bpp YUV planar, 537600 bytes [xv] dx: 0 dy: 0 dw: 800 dh: 448 XXX initial v_pts=0.000 a_pos=28000 (0.500) 1 ??% ??% ??,?% 0 0 *** [ass] Allocating mp_image_t, 800x448x12bpp YUV planar, 537600 bytes *** [vo] Allocating mp_image_t, 800x448x12bpp YUV planar, 537600 bytes *** [menu] Direct Rendering mp_image_t, 800x448x12bpp YUV planar, 537600 bytes Neither PlayResX nor PlayResY defined. Using screen dimensions. fontconfig_select: using default font: (Arial) -> /home/dominik/.mplayer/subfont.ttf, 0 Error opening font: /home/dominik/.mplayer/subfont.ttf, 0 normal text width: 740 0.001 ct: 0.012 73/ 73 36% 21% 7.2% 14 0 Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1208174912 (LWP 18098)] 0x0820125d in ass_render_event (event=0x8bdc134) at ass_render.c:1099 1099 device_y += (render_context.face->size->metrics.descender >> 6); // descender is negative for 'normal' text (gdb) bt #0 0x0820125d in ass_render_event (event=0x8bdc134) at ass_render.c:1099 #1 0x08142b6d in put_image (vf=0x8b8ac48, mpi=0x8d2ca20, pts=-9.2233720368547758e+18) at vf_ass.c:131 #2 0x080ffaed in decode_video (sh_video=0x8b69ed8, start=0x8db3480 "", in_size=403, drop_frame=0, pts=-9.2233720368547758e+18) at dec_video.c:339 #3 0x080a33a3 in main (argc=5, argv=0xbfce1fc4) at mplayer.c:3562 (gdb) disp render_context.face 1: render_context.face = 0x0 (gdb) q The program is running. Exit anyway? (y or n) y R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Tuesday 04 April 2006 00:26, Dominik 'Rathann' Mierzejewski wrote:
Still segfaults. Can't reproduce it. Maybe this will help ?
On Tuesday, 04 April 2006 at 02:59, Evgeniy Stepanov wrote:
On Tuesday 04 April 2006 00:26, Dominik 'Rathann' Mierzejewski wrote:
Still segfaults. Can't reproduce it. Maybe this will help ?
It does. No segfaulting anymore. :) Still, if I don't have ~/.mplayer/subfont.ttf and the sub uses some font I don't have, no subs will be displayed. Why? Of course, symlinking subfont.ttf to some font file makes it work, but I didn't compile in fontconfig support to still have to keep subfont.ttf. Also the fonts are tiny and barely readable. Is there any way to control their size? Other than that, it seems to work fine with the samples I have. Good work! Regards, R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
Dnia niedziela, 2 kwietnia 2006 03:49, Evgeniy Stepanov napisał:
On Sunday 02 April 2006 05:26, Rich Felker wrote:
2) The filter can just handle YV12, which is bad, it should at least be able to handle the most comon YUV formats.
I don't see how that matters too much. No one uses any format except 4:2:0 planar, especially with subtitles.
-vo png, for example, wants something RGB-like. Sure, it's usually not performance-critical, and -vf scale helps. But still an inconvenience. Also, it should work considerably faster with RGB and 4:4:4 planar formats.
The latest patch contained a bug, corrected version attached.
Current ass filter has problem witch Duvall font. Second line of subtitle is too high, so lines are mixed together. Following screenshot explains that: http://home.autocom.pl/albert/sub2line.jpg Archive witch sample video,subs and font is avaliable here: http://home.autocom.pl/albert/renderbug.tar.bz2 -- Piotr Stachura
On Monday 03 April 2006 15:39, Piotr wrote:
Current ass filter has problem witch Duvall font. Second line of subtitle is too high, so lines are mixed together. Following screenshot explains that: http://home.autocom.pl/albert/sub2line.jpg Archive witch sample video,subs and font is avaliable here: http://home.autocom.pl/albert/renderbug.tar.bz2 MPlayer and ftview have the same problem. Font metrics are clearly wrong. Under Windows it is displayed properly. A bug in FreeType ?
Hi, On Saturday 01 April 2006 06:09, Evgeniy Stepanov wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
A minor problem: When work with a truetype font with embeded bitmaps (to improve the visual result at certain fontsize), Sometimes it will give a "Unsupported pixel mode: 1" error. I guess it's because freetype loaded an embeded bitmap instead of a glyph. -- Best Regards, LR
On Monday 03 April 2006 15:03, 陆然 wrote:
A minor problem: When work with a truetype font with embeded bitmaps (to improve the visual result at certain fontsize), Sometimes it will give a "Unsupported pixel mode: 1" error. I guess it's because freetype loaded an embeded bitmap instead of a glyph. Supporting embedded bitmaps would require quite a bit of work, this patch disables them.
Hi
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
The patch also adds attachment support to matroska demuxer, as it was needed to extract fonts that usually come with subtitles. Following sample causes segfault. It happends with and without '-vf ass'
Dnia sobota, 1 kwietnia 2006 00:09, Evgeniy Stepanov napisał: option. I can't remember but perhaps subtitles in file are compressed, remuxing file (w/o compression) causes correct play. It isn't happend without patch! http://home.autocom.pl/albert/db.mkv mplayer: ass.c:392: ass_process_line: Assertion `ass_tracks_info[tid].event_format' failed. Program received signal SIGABRT, Aborted. [Switching to Thread 16384 (LWP 20559)] 0x00002aaaae3cdad9 in kill () from /lib/libc.so.6 backtrace: #0 0x00002aaaae3cdad9 in kill () from /lib/libc.so.6 #1 0x00002aaaabb5d841 in pthread_kill () from /lib/libpthread.so.0 #2 0x00002aaaabb5dbc2 in raise () from /lib/libpthread.so.0 #3 0x00002aaaae3cd7d2 in raise () from /lib/libc.so.6 #4 0x00002aaaae3ceb12 in abort () from /lib/libc.so.6 #5 0x00002aaaae3c6fd0 in __assert_fail () from /lib/libc.so.6 #6 0x0000000000553405 in ass_process_line () #7 0x000000000052f0ee in demux_mkv_fill_buffer () #8 0x00000000004e8f2c in ds_fill_buffer () #9 0x00000000004e90ee in demux_read_data () #10 0x0000000000805ec2 in MP3_DecodeFrame () #11 0x0000000000479e08 in decode_audio () #12 0x0000000000415e4a in main () -- Piotr Stachura
On Wednesday 05 April 2006 19:18, Piotr Stachura wrote:
Following sample causes segfault. It happends with and without '-vf ass' option. I can't remember but perhaps subtitles in file are compressed, remuxing file (w/o compression) causes correct play. It isn't happend without patch!
http://home.autocom.pl/albert/db.mkv
mplayer: ass.c:392: ass_process_line: Assertion `ass_tracks_info[tid].event_format' failed.
Subs in this file are not compressed. Their header (CodecPrivate) is broken, it lacks "[Events]" and subsequent "Format: ..." lines. What program do you use for remuxing ? mkvmerge only allows compression of vobsub subtitles. And it does not add missing lines.
BTW. could you add a font factor size. Currently some of subtitles are so small :/
Something like -vf ass=font-scale=1.5:another-option=value:sub=1.ass ? Not very readable :( There is no such thing as per-filter global options ?
Dnia środa, 5 kwietnia 2006 19:18, Evgeniy Stepanov napisał:
On Wednesday 05 April 2006 19:18, Piotr Stachura wrote:
Following sample causes segfault. It happends with and without '-vf ass' option. I can't remember but perhaps subtitles in file are compressed, remuxing file (w/o compression) causes correct play. It isn't happend without patch!
http://home.autocom.pl/albert/db.mkv
mplayer: ass.c:392: ass_process_line: Assertion `ass_tracks_info[tid].event_format' failed.
Subs in this file are not compressed. Their header (CodecPrivate) is broken, it lacks "[Events]" and subsequent "Format: ..." lines. What program do you use for remuxing ? mkvmerge only allows compression of vobsub subtitles. And it does not add missing lines.
Sample was simple stream cut, not remuxed. I don't know what you talking about missing lines. File oryginally was muxed using mkvmerge (v0.9.5). Remuxing using mkvmerge (v1.6.5) fixed problem but it doesn't explain why mplayer segfaults.
BTW. could you add a font factor size. Currently some of subtitles are so small :/
Something like -vf ass=font-scale=1.5:another-option=value:sub=1.ass ? Not very readable :( There is no such thing as per-filter global options ?
Yes, something like that :) -- Piotr Stachura
On Wednesday 05 April 2006 22:33, Piotr Stachura wrote:
Sample was simple stream cut, not remuxed. I don't know what you talking about missing lines. File oryginally was muxed using mkvmerge (v0.9.5). Remuxing using mkvmerge (v1.6.5) fixed problem but it doesn't explain why mplayer segfaults.
The reason is an ancient mkvmerge version. It does not put those lines into CodecPrivate section. Newer mkvextract contain a workaround for this. Here is an improved patch. Also, -fontconfig and -font options are used for selecting default font, border width is taken from style description. On Monday 03 April 2006 07:24, Alexander Strange wrote:
* This file asserts out: http://raspberryheaven.net/~adtrw/anime/ Makoto%20Shinkai%20-%20Other%20Worlds.mkv ass_render.c:1200: failed assertion `frame_context.dmpi->stride[1] == frame_context.dmpi->chroma_width'
Also fixed.
On Wed, 2006-04-05 at 23:57 +0400, Evgeniy Stepanov wrote:
Here is an improved patch.
The end time for subtitles is when they should disappear already, so the "(ass_current_timecode <= (event->Start + event->Duration))" test in render_frame should use "<" instead of "<=". The fonts seem too small, I get a better match with some screenshots from Windows by setting the font_size_coeff value to .8 instead of .6 (though I'd like to know why a multiplier is needed at all). There is no code path where it wouldn't complain about missing PlayResX/PlayResY (if both are set then it incorrectly says PlayResX is missing). It seems the FT_Set_Char_Size calls do not take PlayResX/PlayResY into account (the first of them calculates a "size" variable using PlayResX but then doesn't do anything with the result). However the subtitles I've tested have had PlayRes equal to the video dimensions so that is not the cause of the too small looking font sizes.
On Tue, Apr 18, 2006 at 02:59:02AM +0300, Uoti Urpala wrote:
On Wed, 2006-04-05 at 23:57 +0400, Evgeniy Stepanov wrote:
Here is an improved patch.
The end time for subtitles is when they should disappear already, so the "(ass_current_timecode <= (event->Start + event->Duration))" test in render_frame should use "<" instead of "<=".
For floats < and <= might as well be identical I think.. :) Rich
On Mon, 2006-04-17 at 21:28 -0400, Rich Felker wrote:
The end time for subtitles is when they should disappear already, so the "(ass_current_timecode <= (event->Start + event->Duration))" test in render_frame should use "<" instead of "<=".
For floats < and <= might as well be identical I think.. :)
I noticed it because two subtitles did incorrectly overlap in a few places. Those aren't floats but integers in milliseconds. And even if those values were kept in floats it almost certainly would still make a difference whether it's < or <=. I found another problem with the patch: if PlayResX+Y are missing, the right defaults (or at least ones matching what Windows video filters use) are apparently 384/288, not the dimensions of the video.
On Tue, Apr 18, 2006 at 05:04:47AM +0300, Uoti Urpala wrote:
On Mon, 2006-04-17 at 21:28 -0400, Rich Felker wrote:
The end time for subtitles is when they should disappear already, so the "(ass_current_timecode <= (event->Start + event->Duration))" test in render_frame should use "<" instead of "<=".
For floats < and <= might as well be identical I think.. :)
I noticed it because two subtitles did incorrectly overlap in a few places. Those aren't floats but integers in milliseconds. And even if those values were kept in floats it almost certainly would still make a difference whether it's < or <=.
I assumed ass_current_timecode was a timestamp from the demuxer which almost surely would not match the low-denominator diadic rationals in the subtitle file. Anyway if these are integer types, I hope they're long long... Rich
ass_current_timecode is long long, in milliseconds. Should be more than enough precision. Here is a better patch. Features and improvements: * Filter options: font-size-multiplier:line-spacing:top-margin:bottom-margin No font size multiplier by default. * External subs file name is taken from -sub option * More precise glyph positioning * More tags supported * DR support * Line breaking for CJK. * Some bugfixes Is the patch acceptable, and if not, what needs to be done ?
Hello,
Is the patch acceptable, and if not, what needs to be done ?
Very quick review:
-D_GNU_SOURCE \ + $(FREETYPE_INC)
There should be a \ at the end of the line as well (so that future patches do not need to change this line). Code should be documentated as described in DOCS/tech/code-documentation.txt, i.e. doxygen comments at least for every global function/variable, and IMHO also for local (i.e. "static" functions. Well, it probably doesn't make sense to comment the default filter-functions, but I guess you get the idea ;-) You should not mix tabs and space for indentation (ass_style_t in libsub/ass_types.h looks particularly bad) - I personally prefer spaces, they are IMO less likely to break, but that's you decision. Variable declarations must be at the beginning of a block (see e.g. ass_draw_bitmap, will not compile with gcc-2.95 otherwise). The stub functions in ass.c might be cleaner to do via defines in ass.h (wouldn't cause useless function calls etc.), not sure about it though. The utf8 conversion functions (ass_utils.c) are, to be honest, butt-ugly. But that is simple to fix later on. Though one comment on it, why do you pass a pointer to utf8_charlen instead of just the char? Two things I was wondering about but couldn't find out myself: 1) do the changes to demux_mkv.c cause any problems for the current subtitle code? I.e. if this patch is applied, will there be any differences between what the old and the new subtitle code support except the extended things like color, positioning etc.? 2) how difficult would it be to make a vo support this? I think for this it would be preferable if only the ass_draw_bitmap was in the filter and the rendering would be done some place outside, with the ass_draw_bitmap function being called via a VF_ message. Such a drawing function would be easy to implement in the vos as well, and vf_vo could pass such a message on to the vo if none of the filters accepted it. Thus there shouldn't be any need for special cases. Or is there too much data that the other functions currently in vf_ass need and is only available there? Greetings, Reimar Döffinger
On Wednesday 19 April 2006 17:59, Reimar Döffinger wrote:
The stub functions in ass.c might be cleaner to do via defines in ass.h (wouldn't cause useless function calls etc.), not sure about it though.
You mean ass_find_sub, ass_change_track ? Or STRVAL, COLORVAL, etc. ? Don't understand your idea.
The utf8 conversion functions (ass_utils.c) are, to be honest, butt-ugly. But that is simple to fix later on. Though one comment on it, why do you pass a pointer to utf8_charlen instead of just the char?
No idea ;) I've taken them from openldap and didn't change much.
Two things I was wondering about but couldn't find out myself: 1) do the changes to demux_mkv.c cause any problems for the current subtitle code? I.e. if this patch is applied, will there be any differences between what the old and the new subtitle code support except the extended things like color, positioning etc.?
Old code should work exactly as it did before. Some info is passed from demuxer to the new code, but nothing is modified.
2) how difficult would it be to make a vo support this? I think for this it would be preferable if only the ass_draw_bitmap was in the filter and the rendering would be done some place outside, with the ass_draw_bitmap function being called via a VF_ message. Such a drawing function would be easy to implement in the vos as well, and vf_vo could pass such a message on to the vo if none of the filters accepted it. Thus there shouldn't be any need for special cases. Or is there too much data that the other functions currently in vf_ass need and is only available there?
We can render subtitles from somewhere before decode_video() into an alpha buffer, and blend it into image later (in vf_ass or some vo_). But would not blending a large (full video dimensions in the worst case) image be too slow ? What is the possible gain of this ?
On Wed, 2006-04-19 at 23:32 +0400, Evgeniy Stepanov wrote:
On Wednesday 19 April 2006 17:59, Reimar Döffinger wrote:
The stub functions in ass.c might be cleaner to do via defines in ass.h (wouldn't cause useless function calls etc.), not sure about it though.
You mean ass_find_sub, ass_change_track ? Or STRVAL, COLORVAL, etc. ? Don't understand your idea.
I think he meant the dummy functions for the "#ifndef HAVE_FREETYPE" case, to define those as empty macros in the header instead.
Hi, On Wed, Apr 19, 2006 at 11:32:13PM +0400, Evgeniy Stepanov wrote:
On Wednesday 19 April 2006 17:59, Reimar Döffinger wrote:
The stub functions in ass.c might be cleaner to do via defines in ass.h (wouldn't cause useless function calls etc.), not sure about it though.
You mean ass_find_sub, ass_change_track ?
I mean void ass_find_sub(unsigned long long timecode) {} in ass.c vs. #define ass_find_sub(l) in ass.h
2) how difficult would it be to make a vo support this? I think for this it would be preferable if only the ass_draw_bitmap was in the filter and the rendering would be done some place outside, with the ass_draw_bitmap function being called via a VF_ message. Such a drawing function would be easy to implement in the vos as well, and vf_vo could pass such a message on to the vo if none of the filters accepted it. Thus there shouldn't be any need for special cases. Or is there too much data that the other functions currently in vf_ass need and is only available there?
We can render subtitles from somewhere before decode_video() into an alpha buffer, and blend it into image later (in vf_ass or some vo_). But would not blending a large (full video dimensions in the worst case) image be too slow ? What is the possible gain of this ?
You misunderstood. Not a full image, just the same size as ass_draw_bitmap. Or more precisely, the idea is: move everything except the ass_draw_bitmap function out of vf_ass and into libass. There, place all ass_draw_bitmap calls by something along the lines vf->control(vf, VFCTRL_PERIODIC_UPDATE, &struct_with_draw_params); (a helper function similar to af_control_any_rev might be helpful). I guess this is not a requirement, but with the current design I fear it might be very hard to extend. I also admit, that with my proposed changes, we almost get a second vf_osd... Greetings, Reimar Döffinger
On Thursday 20 April 2006 00:14, Reimar Döffinger wrote:
2) how difficult would it be to make a vo support this? I think for this it would be preferable if only the ass_draw_bitmap was in the filter and the rendering would be done some place outside, with the ass_draw_bitmap function being called via a VF_ message. Such a drawing function would be easy to implement in the vos as well, and vf_vo could pass such a message on to the vo if none of the filters accepted it. Thus there shouldn't be any need for special cases. Or is there too much data that the other functions currently in vf_ass need and is only available there?
We can render subtitles from somewhere before decode_video() into an alpha buffer, and blend it into image later (in vf_ass or some vo_). But would not blending a large (full video dimensions in the worst case) image be too slow ? What is the possible gain of this ?
You misunderstood. Not a full image, just the same size as ass_draw_bitmap. Or more precisely, the idea is: move everything except the ass_draw_bitmap function out of vf_ass and into libass. There, place all ass_draw_bitmap calls by something along the lines vf->control(vf, VFCTRL_PERIODIC_UPDATE, &struct_with_draw_params); (a helper function similar to af_control_any_rev might be helpful). I guess this is not a requirement, but with the current design I fear it might be very hard to extend. I also admit, that with my proposed changes, we almost get a second vf_osd...
Libass generates series of small bitmaps and passes them to vf_ass. vf_ass keeps this bitmaps and blends them into every frame passing through it. Did I get it right this time ? :) This is surely possible. ass_draw_bitmap needs only a bitmap (single alpha plane), a color, and a point where to put it. Clipping code can be moved to, say, render_text. All the data needed by (libass minus ass_draw_bitmap) is kept in libass' static vars, and is accessible from anywhere.
Hi, On Thu, Apr 20, 2006 at 02:37:53AM +0400, Evgeniy Stepanov wrote:
Libass generates series of small bitmaps and passes them to vf_ass. vf_ass keeps this bitmaps and blends them into every frame passing through it. Did I get it right this time ? :)
Yes, that is it - more or less like the corrent subtitle code works.
This is surely possible. ass_draw_bitmap needs only a bitmap (single alpha plane), a color, and a point where to put it. Clipping code can be moved to, say, render_text. All the data needed by (libass minus ass_draw_bitmap) is kept in libass' static vars, and is accessible from anywhere.
I think this should make it rather simple to implement it in the vos or wherever. Of course, this is just a theory, you probably know best if it's worth the effort and likely to work well enough. Btw. from your description of ass_draw_bitmap it sounds like the vo_gl OSD code already has all the necessary abilities (only the glColor4ub call needs to be moved so that each OSD part can be coloured independantly), so in case you implement it like this I'll try my best to make it work with vo_gl. Greetings, Reimar Döffinger
On Wed, 2006-04-19 at 15:59 +0200, Reimar Döffinger wrote:
Code should be documentated as described in DOCS/tech/code-documentation.txt, i.e. doxygen comments at least for every global function/variable, and IMHO also for local (i.e. "static" functions.
That file claims there should be comments for "every function, no matter whether it is globally or just locally used". No code in MPlayer really follows that, nor should it IMO (commenting obvious things is just clutter, more likely to cause confusion as the code in question changes than to help).
2) how difficult would it be to make a vo support this? I think for this
I think working on this would be better done after the current version of the patch has been applied first.
Hi, On Wed, Apr 19, 2006 at 10:38:21PM +0300, Uoti Urpala wrote:
On Wed, 2006-04-19 at 15:59 +0200, Reimar Döffinger wrote:
Code should be documentated as described in DOCS/tech/code-documentation.txt, i.e. doxygen comments at least for every global function/variable, and IMHO also for local (i.e. "static" functions.
That file claims there should be comments for "every function, no matter whether it is globally or just locally used". No code in MPlayer really
I think most of the code I added follows it except the standard function where the documentation would be the same for all files. But it reminds me, I think I forgot to do it for demux_mpc.c
follows that, nor should it IMO (commenting obvious things is just clutter, more likely to cause confusion as the code in question changes than to help).
Commenting in one line what a function does and a single line for every parameter and the return value should be possible with almost every function without stating anything obvious. Do you know how often I had to read lots of code just to find out if a return value of 0, 1, or -1 means there was an error? A single line can save a lot of effort. Greetings, Reimar Döffinger
Hi, On Wed, Apr 19, 2006 at 10:38:21PM +0300, Uoti Urpala wrote:
On Wed, 2006-04-19 at 15:59 +0200, Reimar Döffinger wrote:
2) how difficult would it be to make a vo support this? I think for this
I think working on this would be better done after the current version of the patch has been applied first.
Well, when the architecture does not make this impossible without rewriting everything, fixing it after applying is a bit late. I do not ask for him to implement this, certainly not, just to consider if it is possible currently. Greetings, Reimar Döffinger
On Wednesday 19 April 2006 17:59, Reimar Döffinger wrote:
Very quick review:
-D_GNU_SOURCE \ + $(FREETYPE_INC)
There should be a \ at the end of the line as well (so that future patches do not need to change this line).
Code should be documentated as described in DOCS/tech/code-documentation.txt, i.e. doxygen comments at least for every global function/variable, and IMHO also for local (i.e. "static" functions. Well, it probably doesn't make sense to comment the default filter-functions, but I guess you get the idea ;-)
You should not mix tabs and space for indentation (ass_style_t in libsub/ass_types.h looks particularly bad) - I personally prefer spaces, they are IMO less likely to break, but that's you decision.
Variable declarations must be at the beginning of a block (see e.g. ass_draw_bitmap, will not compile with gcc-2.95 otherwise).
The stub functions in ass.c might be cleaner to do via defines in ass.h (wouldn't cause useless function calls etc.), not sure about it though.
Fixed.
You now make the global functions empty macros if HAVE_FREETYPE is not defined, but still unconditionally build ass.c which does not have the dummy functions anymore and apparently builds an empty object file. Is there any need to do that? You should add a line like "libsub/libsub.a: $(wildcard libsub/*.[ch])" to the top-level Makefile to trigger recursive make in libsub/ when files there change. @@ -2265,6 +2392,8 @@ default: cont = 1; case MATROSKA_ID_ATTACHMENTS: + cont = demux_mkv_read_attachments (demuxer); + break; case EBML_ID_VOID: ebml_read_skip (s, NULL); break; I think you missed the default case fallthrough above. +demux_mkv_read_attachments (demuxer_t *demuxer) The memory allocated in this function is never freed. Embedded fonts are saved under the user's .mplayer directory, I think there should be at least an option to turn off such creation of new files. Because the created files use names from the video file without any security checks, they can contain things like '../../' and thus new files can be created anywhere on the filesystem. There seems to be some kind of check against overwriting files, I don't see the point of that - it's obviously insufficient as a security check, and if there are different font files with the same name wouldn't you usually want to use the one from the video that was last opened?
On Friday 21 April 2006 06:44, Uoti Urpala wrote:
+demux_mkv_read_attachments (demuxer_t *demuxer)
The memory allocated in this function is never freed.
Attachments are kept in mkv_demuxer_t in the same way as chapters and indices, just for uniformity. This could be used in the future to write fonts to disk only when they are needed, or even to open them directly from memory. If you think this takes too much memory, I can comment it out for now.
Embedded fonts are saved under the user's .mplayer directory, I think there should be at least an option to turn off such creation of new files.
What should be the default value of such option ? Maybe simply not save the fonts when ~/.mplayer/fonts does not exist ?
Because the created files use names from the video file without any security checks, they can contain things like '../../' and thus new files can be created anywhere on the filesystem. There seems to be some kind of check against overwriting files, I don't see the point of that - it's obviously insufficient as a security check, and if there are different font files with the same name wouldn't you usually want to use the one from the video that was last opened?
Agreed.
On Fri, 2006-04-21 at 16:52 +0400, Evgeniy Stepanov wrote:
On Friday 21 April 2006 06:44, Uoti Urpala wrote:
+demux_mkv_read_attachments (demuxer_t *demuxer)
The memory allocated in this function is never freed.
Attachments are kept in mkv_demuxer_t in the same way as chapters and indices, just for uniformity. This could be used in the future to write fonts to disk only when they are needed, or even to open them directly from memory.
If you think this takes too much memory, I can comment it out for now.
It's ok to keep it in memory while the file is being played, but MPlayer can be used to play multiple files without exiting in between. The attachment structures should be freed in demux_close_mkv, like the chapters are. Have you considered playing multiple files elsewhere in the code (I didn't check whether things are reinitialized properly)?
Embedded fonts are saved under the user's .mplayer directory, I think there should be at least an option to turn off such creation of new files.
What should be the default value of such option ? Maybe simply not save the fonts when ~/.mplayer/fonts does not exist ?
As you said, if the attachments are kept in memory anyway you could save them only if/when the subtitles are enabled. I'm not sure what the policy should be otherwise, except that there should at least be some way to disable creating new files.
On Friday 21 April 2006 17:36, Uoti Urpala wrote:
It's ok to keep it in memory while the file is being played, but MPlayer can be used to play multiple files without exiting in between. The attachment structures should be freed in demux_close_mkv, like the chapters are. Have you considered playing multiple files elsewhere in the code (I didn't check whether things are reinitialized properly)?
Playing multiple files more often segfaults than not, even with an unmodified mplayer. When it does not, vf_ass behaves ok.
On Fri, 21 Apr 2006 21:55:26 +0400 Evgeniy Stepanov <eugeni.stepanov@gmail.com> wrote:
Playing multiple files more often segfaults than not, even with an unmodified mplayer. When it does not, vf_ass behaves ok.
Never seen that here, but if nobody report them nor fix them it's not gonna change much. Albeu
The filter does not work correctly with ftp://toro.imec.msu.ru/users/eugeni/1.mkv. The following helps, but I do not completely understand why. static void get_image(struct vf_instance_s* vf, mp_image_t *mpi) { + if(mpi->type==MP_IMGTYPE_IPB) return; if(mpi->flags&MP_IMGFLAG_PRESERVE) return; Updated patch attached.
On Sunday 23 April 2006 14:16, Evgeniy Stepanov wrote:
Updated patch attached.
libsub/libsub.a(ass_render.o)(.text+0xe38): In function `get_glyph': /extra/mplayer/main/libsub/ass_render.c:1022: undefined reference to `FT_Glyph_Stroke' collect2: ld returned 1 exit status make: *** [mplayer] Error 1 Would maybe need a version check for freetype? I seem to have 2.1.7 installed.
On Thursday 27 April 2006 23:49, Jan Knutar wrote:
On Sunday 23 April 2006 14:16, Evgeniy Stepanov wrote:
Updated patch attached.
libsub/libsub.a(ass_render.o)(.text+0xe38): In function `get_glyph': /extra/mplayer/main/libsub/ass_render.c:1022: undefined reference to `FT_Glyph_Stroke' collect2: ld returned 1 exit status make: *** [mplayer] Error 1
Would maybe need a version check for freetype? I seem to have 2.1.7 installed.
Agreed. FT_Glyph_Stroke first appears in 2.1.8rc1. Where is the better place to check this ? Should ./configure print something like "Checking for FreeType >= 2.1.8rc1 (for -vf ass) ... no" , or better put a silent check somewhere in libsub ?
On Fri, Apr 28, 2006 at 01:33:44AM +0400, Evgeniy Stepanov wrote:
On Thursday 27 April 2006 23:49, Jan Knutar wrote:
On Sunday 23 April 2006 14:16, Evgeniy Stepanov wrote:
Updated patch attached.
libsub/libsub.a(ass_render.o)(.text+0xe38): In function `get_glyph': /extra/mplayer/main/libsub/ass_render.c:1022: undefined reference to `FT_Glyph_Stroke' collect2: ld returned 1 exit status make: *** [mplayer] Error 1
Would maybe need a version check for freetype? I seem to have 2.1.7 installed.
Agreed. FT_Glyph_Stroke first appears in 2.1.8rc1. Where is the better place to check this ? Should ./configure print something like "Checking for FreeType >= 2.1.8rc1 (for -vf ass) ... no" , or better put a silent check somewhere in libsub ?
configure Diego
A new patch. - Several bugfixes - New linewrapping algorithm, similar to the one used in libvo/sub.c - 0.8 font size multiplier - sub_step support - -embeddedfonts option disabled by default - Stricter file name validation when saving fonts - Configuration check for freetype version - got rid of most #ifdef's, make does not enter libsub/ if freetype is unavailable or too old
On Apr 29, 2006, at 1:25 PM, Evgeniy Stepanov wrote:
A new patch.
- Several bugfixes - New linewrapping algorithm, similar to the one used in libvo/sub.c - 0.8 font size multiplier - sub_step support - -embeddedfonts option disabled by default - Stricter file name validation when saving fonts - Configuration check for freetype version - got rid of most #ifdef's, make does not enter libsub/ if freetype is unavailable or too old
This is working pretty well now. I think some lines still look kind of different from Windows, possibly because of Shadow, but it's not significant. I had to change two things: * The inability to match any font was apparently a bug in fontconfig (2.3.94), since it works again if I replace FcPatternGet in ass_fontconfig with the typed versions (http://www.fontconfig.org/ fontconfig-devel/r527.html). * I had to disable sub_recode, because it crashed on loading every external sub I tried. Here's a gdb log, although it's not very helpful since optimizations are on: http://astrange.ithinksw.net/vfass_crash_log.txt I'll look more into that.
On Sun, 2006-04-30 at 19:46 -0400, Alexander Strange wrote:
* I had to disable sub_recode, because it crashed on loading every external sub I tried.
If subcp is not set the function uses iconv() on invalid arguments.
On Monday 01 May 2006 03:55, Uoti Urpala wrote:
On Sun, 2006-04-30 at 19:46 -0400, Alexander Strange wrote:
* I had to disable sub_recode, because it crashed on loading every external sub I tried.
If subcp is not set the function uses iconv() on invalid arguments.
A lame mistake indeed. Somehow it works correctly for me, even with valgrind.
On Monday 01 May 2006 03:46, Alexander Strange wrote:
I had to change two things: * The inability to match any font was apparently a bug in fontconfig (2.3.94), since it works again if I replace FcPatternGet in ass_fontconfig with the typed versions (http://www.fontconfig.org/ fontconfig-devel/r527.html).
* I had to disable sub_recode, because it crashed on loading every external sub I tried. Here's a gdb log, although it's not very helpful since optimizations are on: http://astrange.ithinksw.net/vfass_crash_log.txt I'll look more into that.
Patches attached. Do they help ?
On May 1, 2006, at 9:28 AM, Evgeniy Stepanov wrote:
On Monday 01 May 2006 03:46, Alexander Strange wrote:
I had to change two things: * The inability to match any font was apparently a bug in fontconfig (2.3.94), since it works again if I replace FcPatternGet in ass_fontconfig with the typed versions (http://www.fontconfig.org/ fontconfig-devel/r527.html).
* I had to disable sub_recode, because it crashed on loading every external sub I tried. Here's a gdb log, although it's not very helpful since optimizations are on: http://astrange.ithinksw.net/vfass_crash_log.txt I'll look more into that.
Patches attached. Do they help ?
Yes, both problems are fixed now.
Evgeniy Stepanov wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification.
Congratulations for starting the 3rd independent open source SSA/ASS renderer project. And sorry for being slightly pissed by this. If the effort, instead of being splitted to 3 projects, would have gone into one project, we'd by now have a full-fledged ASS renderer, and not three equally unfinished slowly-if-at-all progressing projects. For your reference, there is an overview at http://asa.diac24.net/Compatibility Please feel free to add your project to the list. -David Lamparter
On Thu, 2006-05-04 at 18:24 +0200, David Lamparter wrote:
Evgeniy Stepanov wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification.
Congratulations for starting the 3rd independent open source SSA/ASS renderer project. And sorry for being slightly pissed by this.
If the effort, instead of being splitted to 3 projects, would have gone into one project, we'd by now have a full-fledged ASS renderer, and not three equally unfinished slowly-if-at-all progressing projects.
While not complete, I think his renderer is already pretty good, and it's unfair to characterize it as an "equally unfinished slowly-if-at-all progressing project". He's fixed problems found in it pretty quickly, and I'd expect it to be added to MPlayer in the near future. If you think the other projects might be better candidates for MPlayer SSA/ASS subtitle rendering, you should provide arguments for that; however based on the asa website you mentioned that project does not look like a viable candidate at the moment.
Dnia czwartek, 4 maja 2006 19:42, Uoti Urpala napisał:
On Thu, 2006-05-04 at 18:24 +0200, David Lamparter wrote:
Evgeniy Stepanov wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification.
Congratulations for starting the 3rd independent open source SSA/ASS renderer project. And sorry for being slightly pissed by this.
If the effort, instead of being splitted to 3 projects, would have gone into one project, we'd by now have a full-fledged ASS renderer, and not three equally unfinished slowly-if-at-all progressing projects.
While not complete, I think his renderer is already pretty good, and it's unfair to characterize it as an "equally unfinished slowly-if-at-all progressing project". He's fixed problems found in it pretty quickly, and I'd expect it to be added to MPlayer in the near future. If you think the other projects might be better candidates for MPlayer SSA/ASS subtitle rendering, you should provide arguments for that; however based on the asa website you mentioned that project does not look like a viable candidate at the moment.
I agree, watching movie with this filter I can't say, what subtitles is called "soft" and what are called "hard". It's the best implemetation that I saw working. Anyway I found unimplement? function :P It's a subtitles that are displayed as going info (like on news channel going from right to left). Currently they are boxed to the screen (don't dissapear outside screen but they are going up :/) When I found this movie, I will send screenshot and sample about it. Besides if it's "3rd independent open source SSA/ASS renderer project" then why it's first that is used in popular player?? -- Piotr Stachura
Dnia czwartek, 4 maja 2006 20:56, Piotr Stachura napisał:
Anyway I found unimplement? function :P It's a subtitles that are displayed as going info (like on news channel going from right to left). Currently they are boxed to the screen (don't dissapear outside screen but they are going up :/)
Sorry, current version of patch plays it perfectly, my bad. -- Piotr Stachura
On Thursday 04 May 2006 20:24, David Lamparter wrote:
Congratulations for starting the 3rd independent open source SSA/ASS renderer project. And sorry for being slightly pissed by this.
If the effort, instead of being splitted to 3 projects, would have gone into one project, we'd by now have a full-fledged ASS renderer, and not three equally unfinished slowly-if-at-all progressing projects.
The problem is, I did not know about your project when it all started (the end of february), though I googled a lot. Maybe I should have tried harder. As of now, from the compatibility page I see only one point where Asa is superior to my filter, that is "\fe handling", and a lot of points where vf_ass is better (\clip, \fad, \move, \t and more). And btw, it is not that slow. The development started only 2 months and a week ago.
On Thu, May 04, 2006 at 11:30:38PM +0400, Evgeniy Stepanov wrote:
On Thursday 04 May 2006 20:24, David Lamparter wrote:
Congratulations for starting the 3rd independent open source SSA/ASS renderer project. And sorry for being slightly pissed by this.
If the effort, instead of being splitted to 3 projects, would have gone into one project, we'd by now have a full-fledged ASS renderer, and not three equally unfinished slowly-if-at-all progressing projects.
The problem is, I did not know about your project when it all started (the end of february), though I googled a lot. Maybe I should have tried harder.
As of now, from the compatibility page I see only one point where Asa is superior to my filter, that is "\fe handling", and a lot of points where vf_ass is better (\clip, \fad, \move, \t and more).
And btw, it is not that slow. The development started only 2 months and a week ago.
Don't be discouraged by David's comments. I haven't seen any other SSA/ASS implementation with _ANY_ chance of getting included in MPlayer.. Keep up the good work. Rich
Am Donnerstag, den 04.05.2006, 20:21 -0400 schrieb Rich Felker:
Don't be discouraged by David's comments. I haven't seen any other SSA/ASS implementation with _ANY_ chance of getting included in MPlayer.. Keep up the good work.
I actually have to admit your renderer DOES look pretty good. I didn't look at it before posting my previous mail (I just waited for you to put up the features at the wiki :P)
On Thu, May 04, 2006 at 11:30:38PM +0400, Evgeniy Stepanov wrote:
And btw, it is not that slow. The development started only 2 months and a week ago.
Thing is, aside from the 3 currently active projects, there was quite a number that failed before them. Every single of them seemed ambitious, had great concepts - and then starved off anyway. So "i'm writing an ASS renderer" became a running gag among fansubbers. You'd be the first one to succeed. Either way, sorry for the annoyed post. -David P.S.: One "complaint" I do hold up, though - are you actually getting feedback from fansubbers? If your renderer is off a single bit, the typesetters will roast you ;)
On Friday 05 May 2006 15:21, David Lamparter wrote:
P.S.: One "complaint" I do hold up, though - are you actually getting feedback from fansubbers? If your renderer is off a single bit, the typesetters will roast you ;)
It's probably impossible to make it bit-exact. Besides, that would probably be a bad idea if the renderer gets vo help for speed like the current OSD, then you'd probably prefer the subs rendered at screen resolution rather than rendered at movie resolution. *starts counting how many minutes until Rich "informs" us that karaoke people and typesetting people aren't needed* :)
On Friday 05 May 2006 16:21, David Lamparter wrote:
P.S.: One "complaint" I do hold up, though - are you actually getting feedback from fansubbers? If your renderer is off a single bit, the typesetters will roast you ;)
I am hoping to get heaps of feedback once it gets accepted in mplayer cvs :) Things like shadow and antialiasing will probably never look exactly the same as with VSFilter. But I hope to make it close enough.
On Fri, May 05, 2006 at 02:21:11PM +0200, David Lamparter wrote:
On Thu, May 04, 2006 at 11:30:38PM +0400, Evgeniy Stepanov wrote:
And btw, it is not that slow. The development started only 2 months and a week ago.
Thing is, aside from the 3 currently active projects, there was quite a number that failed before them. Every single of them seemed ambitious, had great concepts - and then starved off anyway. So "i'm writing an ASS renderer" became a running gag among fansubbers. You'd be the first one to succeed.
IMO the problem with the other projects was probably that they were so "ambitious" as you put it, and worried about making a fancy bloated library API rather than just making something that works.. Working alongside MPlayer and developing something that can actually perform well is a much better short-term approach IMO. Then someone can later abstract it into a general-purpose, slow library if desired. :) Rich
Attached patch includes 2 major changes: - -vf ass=::x:y adds margins on top and bottom of the image, and places top- and subtitles there. - Karaoke support (\k, \kf, \K, \ko). How is the reviewing going ? This constant changes must be complicating it. Looks like pre8 is delayed, maybe its time to apply the patch ?
Looks like nobody tried compiling this under MinGW yet. Of course, it fails ;) Fix attached. Problem reported by Victor Farias.
On Saturday, 06 May 2006 at 20:43, Evgeniy Stepanov wrote:
Looks like nobody tried compiling this under MinGW yet. Of course, it fails ;) Fix attached. Problem reported by Victor Farias.
Please don't send "patches for patches", but apply them by yourself and post a whole updated patch instead. This is in the patch submission guidelines, BTW. Thanks, R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Sunday 07 May 2006 21:23, Dominik 'Rathann' Mierzejewski wrote:
Please don't send "patches for patches", but apply them by yourself and post a whole updated patch instead. This is in the patch submission guidelines, BTW.
No problem.
A few problem using \bord. http://www.souju.org/temp/bord-test1.jpg http://www.souju.org/temp/bord-expected1.jpg http://www.souju.org/temp/bord.ass
On Monday 08 May 2006 12:54, Kridsada Krakronkul wrote:
A few problem using \bord.
The output I get is more like "bord-expected1.jpg". Perhaps it depends on the video file (and maybe even font), could you provide samples ?
Evgeniy Stepanov <eugeni.stepanov <at> gmail.com> writes:
On Monday 08 May 2006 12:54, Kridsada Krakronkul wrote:
A few problem using \bord.
The output I get is more like "bord-expected1.jpg". Perhaps it depends on the video file (and maybe even font), could you provide samples ?
Hi, On Saturday 06 May 2006 03:51, Evgeniy Stepanov wrote:
Attached patch includes 2 major changes: - -vf ass=::x:y adds margins on top and bottom of the image, and places top- and subtitles there. - Karaoke support (\k, \kf, \K, \ko).
How is the reviewing going ? This constant changes must be complicating it. Looks like pre8 is delayed, maybe its time to apply the patch ?
The patch has a minor problem with freetype 2.2.1, the prototype of FT_Stroker_New inftstroke.h has changed from FT_EXPORT( FT_Error ) FT_Stroker_New( FT_Memory memory, FT_Stroker *astroker ); to FT_EXPORT( FT_Error ) FT_Stroker_New( FT_Library library, FT_Stroker *astroker ); So the code in the patch should be modified to be aware of the change. And there is also an annoying bug with \fs{negative number}, I don't know if this is legitimate, but the tag will make the system almost hang. At least there should be some sanity check IMHO. -- Best Regards, LR
On Monday 29 May 2006 19:52, 陆然 wrote:
The patch has a minor problem with freetype 2.2.1, the prototype of FT_Stroker_New inftstroke.h has changed from
FT_EXPORT( FT_Error ) FT_Stroker_New( FT_Memory memory, FT_Stroker *astroker );
to
FT_EXPORT( FT_Error ) FT_Stroker_New( FT_Library library, FT_Stroker *astroker );
So the code in the patch should be modified to be aware of the change.
And there is also an annoying bug with \fs{negative number}, I don't know if this is legitimate, but the tag will make the system almost hang. At least there should be some sanity check IMHO.
Both fixed in todays patch.
On Friday 21 April 2006 06:44, Uoti Urpala wrote:
You now make the global functions empty macros if HAVE_FREETYPE is not defined, but still unconditionally build ass.c which does not have the dummy functions anymore and apparently builds an empty object file. Is there any need to do that?
You should add a line like "libsub/libsub.a: $(wildcard libsub/*.[ch])" to the top-level Makefile to trigger recursive make in libsub/ when files there change.
@@ -2265,6 +2392,8 @@ default: cont = 1; case MATROSKA_ID_ATTACHMENTS: + cont = demux_mkv_read_attachments (demuxer); + break; case EBML_ID_VOID: ebml_read_skip (s, NULL); break;
I think you missed the default case fallthrough above.
Fixed.
+demux_mkv_read_attachments (demuxer_t *demuxer)
The memory allocated in this function is never freed.
Fixed.
Embedded fonts are saved under the user's .mplayer directory, I think there should be at least an option to turn off such creation of new files.
-noembeddedfonts
Because the created files use names from the video file without any security checks, they can contain things like '../../' and thus new files can be created anywhere on the filesystem. There seems to be some kind of check against overwriting files, I don't see the point of that - it's obviously insufficient as a security check, and if there are different font files with the same name wouldn't you usually want to use the one from the video that was last opened?
Fixed. Added a short desription to man page. -subcp now supported.
Hi, On Wednesday 19 April 2006 02:36, Evgeniy Stepanov wrote:
Here is a better patch. Features and improvements: * Filter options: font-size-multiplier:line-spacing:top-margin:bottom-margin No font size multiplier by default. * External subs file name is taken from -sub option * More precise glyph positioning * More tags supported
There might be some regression, a test sub is in the attachment, the old code put the sub in the middle of the picture, but with the new code the sub I cannot find the sub anywhere.
* Line breaking for CJK.
When I have a long sentence with only one space in it, the break will accumulate at the space, since the code cannot tell if the space has already been used as a break. And two problem about fontconfig: 1. make 'Arial' the default font is not a good idea IMHO, all the cjk glyphs are missing. 2. the code use strcmp to test if the font provided by fontconfig is the font requested, too bad the comparison is case sensitive. If I specify a font 'arial' in the sub it will never be found. -- Best Regards, LR
On Wednesday 19 April 2006 18:44, 陆然 wrote:
There might be some regression, a test sub is in the attachment, the old code put the sub in the middle of the picture, but with the new code the sub I cannot find the sub anywhere.
* Line breaking for CJK.
When I have a long sentence with only one space in it, the break will accumulate at the space, since the code cannot tell if the space has already been used as a break.
And two problem about fontconfig: 1. make 'Arial' the default font is not a good idea IMHO, all the cjk glyphs are missing. 2. the code use strcmp to test if the font provided by fontconfig is the font requested, too bad the comparison is case sensitive. If I specify a font 'arial' in the sub it will never be found.
Arial is the default according to specification. You can use fontconfig=yes font=<font family> to change this behavior. Attached version also uses fontconfig=no, font=<path> ahead of Arial. Other things you mentioned are also fixed.
Hi, On Thursday 20 April 2006 03:45, Evgeniy Stepanov wrote:
Arial is the default according to specification. You can use fontconfig=yes font=<font family> to change this behavior. Attached version also uses fontconfig=no, font=<path> ahead of Arial.
With windows this is ok since it will fallback to other fonts if the required glyph cannot be found in Arial. Can this be achieved in mplayer? Anyway the current behavior is good enough IMHO.
Other things you mentioned are also fixed.
The patch works great, thanks. -- Best Regards, LR
Hi ??????! On 2006.04.20 at 10:20:20 +0800, ?????? wrote next:
On Thursday 20 April 2006 03:45, Evgeniy Stepanov wrote:
Arial is the default according to specification. You can use fontconfig=yes font=<font family> to change this behavior. Attached version also uses fontconfig=no, font=<path> ahead of Arial.
With windows this is ok since it will fallback to other fonts if the required glyph cannot be found in Arial. Can this be achieved in mplayer? Anyway the current behavior is good enough IMHO.
I always thought that it's one of the main purpuses of fontconfig, to calculate the best matching font for the symbols that don't exist in current... I don't know fontconfig, but looking at the docs, it looks like you need to call FcFontRenderPrepare or something like that and get a a list of similar fonts to the one selected and use it. Without fontconfig, this task is impossible for sure. -- Vladimir
Hi, On Thursday 20 April 2006 13:50, Vladimir Mosgalin wrote:
I always thought that it's one of the main purpuses of fontconfig, to calculate the best matching font for the symbols that don't exist in current...
I don't know fontconfig, but looking at the docs, it looks like you need to call FcFontRenderPrepare or something like that and get a a list of similar fonts to the one selected and use it.
Does this mean that we should check every char for a list of fonts? I guess that would be too expensive for sub rendering. -- Best Regards, LR
Hi ??????! On 2006.04.20 at 16:57:02 +0800, ?????? wrote next:
I always thought that it's one of the main purpuses of fontconfig, to calculate the best matching font for the symbols that don't exist in current...
I don't know fontconfig, but looking at the docs, it looks like you need to call FcFontRenderPrepare or something like that and get a a list of similar fonts to the one selected and use it.
Does this mean that we should check every char for a list of fonts? I guess that would be too expensive for sub rendering.
I don't think so. For example, you have "Arial" font, picked by user. You want to render CJK symbol. Fontconfig calculates that "Kochi Gothic" font is closest to "Arial". Now you have "Arial; Kochi Gothic" font list. If next required symbol is in either of them, then they are used (in order); if not, say you want to render some math symbol, another font lookup is performed and now you have three fonts in the list. I haven't really checked it, but according to documentation, it should work this way. -- Vladimir
On Thursday 20 April 2006 16:00, Vladimir Mosgalin wrote:
I don't think so. For example, you have "Arial" font, picked by user. You want to render CJK symbol. Fontconfig calculates that "Kochi Gothic" font is closest to "Arial". Now you have "Arial; Kochi Gothic" font list. If next required symbol is in either of them, then they are used (in order); if not, say you want to render some math symbol, another font lookup is performed and now you have three fonts in the list.
I haven't really checked it, but according to documentation, it should work this way.
Even simpler, single FcFontSort call should do what you want. I think the present behaviour is ok for now. You can set an CJK font as a default mplayer font. Or you could implement this and send me a patch. :)
Hi Evgeniy Stepanov! On 2006.04.18 at 22:36:09 +0400, Evgeniy Stepanov wrote next:
Here is a better patch. Features and improvements: * Filter options: font-size-multiplier:line-spacing:top-margin:bottom-margin No font size multiplier by default. * External subs file name is taken from -sub option
For some reason, it always assumes that file is in utf8 encoding. Of course, mostly it it's not the case.. For example, Russian subs are always in cp1251. It should honor -subcp config option, too. Also, here is an example which produces quite different result from what it should. I'm not talking about cool karaoke effect (don't even expect this to be supported anytime soon ;)), but about incorrect font color. Shadow seem to be quite different from how directvobsub renders it, too. Anyway, many thanks for this filter! PS Any chances it will support sub_step events? It just changes subtitle delay, which works already, but you probably need to explain current subtitles' timings to mplayer. -- Vladimir
On Thursday 20 April 2006 03:48, Vladimir Mosgalin wrote:
PS Any chances it will support sub_step events? It just changes subtitle delay, which works already, but you probably need to explain current subtitles' timings to mplayer. It will someday, don't see any problem with this.
Dnia sobota, 1 kwietnia 2006 00:09, Evgeniy Stepanov napisał:
Hi
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
The patch also adds attachment support to matroska demuxer, as it was needed to extract fonts that usually come with subtitles.
BTW. could you add a font factor size. Currently some of subtitles are so small :/ -- Piotr Stachura
Hi, On Saturday 01 April 2006 06:09, Evgeniy Stepanov wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
I tested the patch with the ass file in the attachment, a noticable problem is a lot of "bad color at ass_render.c" error. And this subtitle is really fancy, I assume there are some other problems but I have not found a chance to watch it on a windows box with vobsub. So I don't know those abnormal behaviors are bugs in the code or bugs in the sub file. -- Best Regards, LR
On Sunday 09 April 2006 12:19, 陆然 wrote:
I tested the patch with the ass file in the attachment, a noticable problem is a lot of "bad color at ass_render.c" error. And this subtitle is really fancy, I assume there are some other problems but I have not found a chance to watch it on a windows box with vobsub. So I don't know those abnormal behaviors are bugs in the code or bugs in the sub file. This script uses some rare and (yet) unimplemented tags. "bad color" refers to {\1a&HFF}, which, according to specs, should be written as {\1a&HFF&}. Not a problem, both formats could be supported. Thanks for providing such a complex script.
Hi, On Saturday 01 April 2006 06:09, Evgeniy Stepanov wrote:
Here is an attempt to add ASS rendering capability to mplayer. It is designed as a filter and supports a large part of ASS so-called specification. Reads both external subtitles and the ones embedded in matroska.
I've just found the code have the same linebreak problem with CJK subtitles like the old sub code. There is no 'Space' in CJK, so the 'last_space' in 'ass_render_event' will always be -1, and there will be no break. A simple workaround could be breaking at the last char if no space is found. Or maybe we could do some fancy stuff like detecting the charset of the subtitle and handle it seperately, as discussed in the list some time ago. -- Best Regards, LR
Hi Attached is a new version of the patch. The interface has been reworked (take a look at libass/ass.h). It is almost a standalone library now (of course, it still uses mp_msg and stuff). The filter has been moved to libass and cannot be specified on the command line. Instead, -ass switch should be used. Filter is automatically added to the chain, like vf_menu. It's options have become global, -ass-font-scale and so on. Lots of comments, some minor bugfixes, a bit of code cleanup. Autosubs are now supported. The architecture is now like Reimar suggested: ass_render_frame() produces a list of bitmaps, that are blended later by vf_ass. Font matching has been modified. Default font is only used when fontconfig is unavailable. Otherwise, it accepts any font chosen by fontconfig. Please test and review. Any comments/suggestions are welcome.
On Wed, 2006-05-31 at 22:28 +0400, Evgeniy Stepanov wrote:
The architecture is now like Reimar suggested: ass_render_frame() produces a list of bitmaps, that are blended later by vf_ass.
I think his suggestion wasn't quite that, but rather that vf_ass create the list of bitmaps without necessarily blending them to the image, which could be done inside the vo instead in some cases. This might avoid an extra copy of the frame in case the original image vf_ass gets cannot be changed, but the vo needs to do a copy internally anyway and that copy can be changed before displaying it. The way you currently implement it won't get the timecodes quite right (though it's hard to see when they're not right anyway in the current MPlayer - I intended to improve that when CVS went down and will probably continue some time after SVN is finally up). You should code it so that the time of current frame used by subtitle rendering can be read from the pts argument to vf_ass (though that argument isn't yet set in current unpatched MPlayer). +.B \-ass-top-margin <value> +Adds a black band at the top of the frame. SSA/ASS renderer will place toptitles there +.B \-ass-bottom-margin <value> +Adds a black band at the bottom of the frame. SSA/ASS renderer will place subtitles there. Any reason to have these separately from the existing "-vf expand" functionality? How do they interact with it?
Hi, On Sun, Jun 04, 2006 at 09:54:50AM +0300, Uoti Urpala wrote:
On Wed, 2006-05-31 at 22:28 +0400, Evgeniy Stepanov wrote:
The architecture is now like Reimar suggested: ass_render_frame() produces a list of bitmaps, that are blended later by vf_ass.
I think his suggestion wasn't quite that, but rather that vf_ass create the list of bitmaps without necessarily blending them to the image, which could be done inside the vo instead in some cases.
No, I think he understood me right. Though instead of using the global vo_ass variable I had thought about using a VF_CONTROL message. But I can't say which approach really is better. It would have some advantages with audio filters (inserting the filter when it is needed the first time, as done with -af volume when there is no hardware mixer available), but the video filter chain is much less flexible so it might not be worth it here.
This might avoid an extra copy of the frame in case the original image vf_ass gets cannot be changed, but the vo needs to do a copy internally anyway and that copy can be changed before displaying it.
Hmm. As I understood the code, you can just leave vf_ass out and it should happen that way. Though I admit I didn't look closely at the auto-loading code - in the future it would probably have to check if teh vo already suuports the functionality and not load vf_ass then.
The way you currently implement it won't get the timecodes quite right (though it's hard to see when they're not right anyway in the current MPlayer - I intended to improve that when CVS went down and will probably continue some time after SVN is finally up). You should code it so that the time of current frame used by subtitle rendering can be read from the pts argument to vf_ass (though that argument isn't yet set in current unpatched MPlayer).
Hmm... maybe I misunderstood, but I though vo_ass just contained the subtitles for the currently rendered frame... Well, I guess it might break a bit when using filters that introduce a delay, e.g. -vf tile or -vf ivtc or so. Btw. the vf_ass variable is defined globally in mplayer.c, but used only in one place, so I guess it could be moved. Greetings, Reimar Döffinger
On Sun, 2006-06-04 at 12:02 +0200, Reimar Döffinger wrote:
On Sun, Jun 04, 2006 at 09:54:50AM +0300, Uoti Urpala wrote:
On Wed, 2006-05-31 at 22:28 +0400, Evgeniy Stepanov wrote:
The architecture is now like Reimar suggested: ass_render_frame() produces a list of bitmaps, that are blended later by vf_ass.
I think his suggestion wasn't quite that, but rather that vf_ass create the list of bitmaps without necessarily blending them to the image, which could be done inside the vo instead in some cases.
No, I think he understood me right. Though instead of using the global vo_ass
The difference here mainly seems to be when the bitmaps are created. The current way of creating them at the end of the main loop outside the filter chain is certainly wrong; even if there are no filters which would affect the timestamps, the bitmaps will be rendered even if the next frame will be dropped and you can only get the correct (next) timestamp if you rely on knowing it before reading the frame from the demuxer. I think they should be rendered in vf_ass as part of the filter chain, or if there's a reason to avoid having a filter appear in the chain at all then the special case for them should be at the _end_ of the chain (and in any case at least between decoding a frame and calling vo flip).
The way you currently implement it won't get the timecodes quite right (though it's hard to see when they're not right anyway in the current MPlayer - I intended to improve that when CVS went down and will probably continue some time after SVN is finally up). You should code it so that the time of current frame used by subtitle rendering can be read from the pts argument to vf_ass (though that argument isn't yet set in current unpatched MPlayer).
Hmm... maybe I misunderstood, but I though vo_ass just contained the subtitles for the currently rendered frame... Well, I guess it might break a bit when using filters that introduce a delay, e.g. -vf tile or -vf ivtc or so.
Now it renders the bitmaps using sh_video->pts as the time _after_ vo flip has already been called for the image with that pts, so it's wrong even if there are no special filters. But as I mentioned above sh_video->pts is currently somewhat wrong anyway, unless you use the -correct-pts option in the patch I posted earlier.
On Sunday 04 June 2006 14:55, Uoti Urpala wrote:
Now it renders the bitmaps using sh_video->pts as the time _after_ vo flip has already been called for the image with that pts, so it's wrong even if there are no special filters. But as I mentioned above sh_video->pts is currently somewhat wrong anyway, unless you use the -correct-pts option in the patch I posted earlier.
The timecodes in the filter are supposed to be correct, at least the most close to correct I can get in mplayer ? A good reason to move rendering back to the filter. So, it (filter) will render subtitles and then either blend the resulting bitmaps, or pass them to the vo, depending on VFCAP.
+.B \-ass-top-margin <value> +Adds a black band at the top of the frame. SSA/ASS renderer will place toptitles there
+.B \-ass-bottom-margin <value> +Adds a black band at the bottom of the frame. SSA/ASS renderer will place subtitles there.
Any reason to have these separately from the existing "-vf expand" functionality? How do they interact with it?
Renderer needs to know the exact placement of original image inside the frame. Expand filter does not share such information. This breaks subtitle positioning. ass-*-margin adds a black band to the frame (impemented in vf_ass) and passes its height to the renderer. Maybe it would be better to store vf_expand args in some global variables and use them instead. The same probably needs to be done with vf_crop. Generally, there is no way to automatically determine correct image placement withing the frame. Usually, vf_expand is used to add black bands, and vf_crop - to remove then. Looks like the best guess is to use frame dimensions after crop, but before expand for subtitle positioning.
On Sun, 2006-06-04 at 16:54 +0400, Evgeniy Stepanov wrote:
On Sunday 04 June 2006 14:55, Uoti Urpala wrote:
Now it renders the bitmaps using sh_video->pts as the time _after_ vo flip has already been called for the image with that pts, so it's wrong even if there are no special filters. But as I mentioned above sh_video->pts is currently somewhat wrong anyway, unless you use the -correct-pts option in the patch I posted earlier.
The timecodes in the filter are supposed to be correct, at least the most close to correct I can get in mplayer ? A good reason to move rendering back to the filter. So, it (filter) will render subtitles and then either blend the resulting bitmaps, or pass them to the vo, depending on VFCAP.
The timecodes aren't set in the filter chain at all yet, but it is easy to make the current (somewhat incorrect) timecodes appear there so that an ass-specific workaround like the ass_find_sub() function you had before isn't needed. I think this change to mplayer.c should be enough for that: - blit_frame=decode_video(sh_video,start,in_size,drop_frame, MP_NOPTS_VALUE); + blit_frame=decode_video(sh_video,start,in_size,drop_frame, sh_video->pts); I have a patch that gives accurate timecodes in the filter chain, and with that you could get accurate values outside the chain too. However I think that getting the timecodes from inside the chain is the best solution, as that makes it possible to alter the position of the filter within the chain which could matter when the other filters DO manipulate pts. Your current code needs to be changed anyway as it simply uses the timestamps too late, after the corresponding frame has already been shown.
On Sunday 04 June 2006 14:02, Reimar Döffinger wrote:
No, I think he understood me right. Though instead of using the global vo_ass variable I had thought about using a VF_CONTROL message. But I can't say which approach really is better. It would have some advantages with audio filters (inserting the filter when it is needed the first time, as done with -af volume when there is no hardware mixer available), but the video filter chain is much less flexible so it might not be worth it here.
I've tried to initialize the library when it is first needed (for example, user switched to the SSA track), but this takes some time (especially fontconfig initialization) and playback noticably lags. Does not look very good. As for the filter, it can be fixed to have no performance penalty in case when there are no bitmaps to blend. Then it might be better to always add it to the chain.
Hmm. As I understood the code, you can just leave vf_ass out and it should happen that way. Though I admit I didn't look closely at the auto-loading code - in the future it would probably have to check if teh vo already suuports the functionality and not load vf_ass then.
Exactly. I intend to add something like VFCAP_OSD and VFCTRL_DRAW_OSD. But we might still need to load vf_ass if we are going to use video pts from the filter chain.
Hmm... maybe I misunderstood, but I though vo_ass just contained the subtitles for the currently rendered frame... Well, I guess it might break a bit when using filters that introduce a delay, e.g. -vf tile or -vf ivtc or so.
You are correct, it contains bitmaps for a single frame. But now it seems it might be the wrong frame ;) Btw, do I understand this right: the idea of VFCTRL_DRAW_OSD is to tell vf/vo that OSD has changed, and it should be redrawn ? And this is useful for some vo's that use overlays to put osd and therefore don't need to blend it into each frame ?
Hi, On Sun, Jun 04, 2006 at 04:01:19PM +0400, Evgeniy Stepanov wrote:
On Sunday 04 June 2006 14:02, Reimar Döffinger wrote:
Hmm. As I understood the code, you can just leave vf_ass out and it should happen that way. Though I admit I didn't look closely at the auto-loading code - in the future it would probably have to check if teh vo already suuports the functionality and not load vf_ass then.
Exactly. I intend to add something like VFCAP_OSD and VFCTRL_DRAW_OSD. But we might still need to load vf_ass if we are going to use video pts from the filter chain.
Well, another possibility (maybe the same as the other person replying had in mind) would be create the structure to be drawn in vf_ass (as before I guess, so sorry if I made you do some in the end pointless work *g*), then call a VFCTRL_DRAW_EXTENDED_OSD, with a pointer to that struct as argument, and if it fails draw it yourself. But that still leaves the problem that the subtitle might not end up at the frame it should appear. Also there is the problem that some vos can draw the OSD only after they have all of the frame... Another idea would be to just use such a VFCTRL to see if e.g. the vo supports the "extended" OSD (could also be used to check if there is a filter in between vf_ass and the vo that would break thinks, like the mentioned vf_tile), and if yes pass the OSD to be drawn in the mp_image structure (the vo would then also be responsible for freeing the data I guess). That's the best ideas I have so far, but there probably are even better ones to be found *g*
Btw, do I understand this right: the idea of VFCTRL_DRAW_OSD is to tell vf/vo that OSD has changed, and it should be redrawn ? And this is useful for some vo's that use overlays to put osd and therefore don't need to blend it into each frame ?
VFCTRL_DRAW_OSD currently is called every frame, right after the frame has been passed completely through the filter chain to the vo (which also means that vf_expand has to use a different mechanism to draw the OSD - it only "captures" any such message it gets to stop the vo from drawing the OSD as well), regardless if the OSD changed or not, and causes the vf_vo filter to call the draw_osd function of the vo. If the OSD has changed can be queried by calling vo_osd_changed(0), but this tends to be broken from time to time since only vo_gl seems to use it. In theory VFCTRL_DRAW_OSD could be used to update the OSD while the frame is still the same, but in practice with will cause a corrupted picture with anything except vo_gl. Greetings, Reimar Döffinger
On Sunday 04 June 2006 23:18, Reimar Döffinger wrote:
Well, another possibility (maybe the same as the other person replying had in mind) would be create the structure to be drawn in vf_ass (as before I guess, so sorry if I made you do some in the end pointless work *g*), then call a VFCTRL_DRAW_EXTENDED_OSD, with a pointer to that struct as argument, and if it fails draw it yourself. But that still leaves the problem that the subtitle might not end up at the frame it should appear.
Still, the filter's pts is the most accurate value one can get in mplayer, as far as I understand. Currently vf_ass is added to the end of the chain (nearest to the vo), so nothing should affect its interaction with vo.
Also there is the problem that some vos can draw the OSD only after they have all of the frame...
They can store the pointer somewhere and use it later, when they get the whole frame. vf_ass can guarantee that it will be valid until the beginning of the next frame. Another idea: vo capable of "extended osd" could call rendering functions from libass itself when it is ready and blend the resulting bitmaps immediately. This could be wrapped in a nice call like get_extended_osd_bitmaps(double pts). vf_ass is only added if vo does not support this.
Another idea would be to just use such a VFCTRL to see if e.g. the vo supports the "extended" OSD (could also be used to check if there is a filter in between vf_ass and the vo that would break thinks, like the mentioned vf_tile), and if yes pass the OSD to be drawn in the mp_image structure (the vo would then also be responsible for freeing the data I guess).
This makes sense only if something between vf_ass and vo is messing with frames. I hope we can avoid this situation completely by adding vf_ass to the very end of the filter chain.
On Mon, 2006-06-05 at 04:21 +0400, Evgeniy Stepanov wrote:
Another idea: vo capable of "extended osd" could call rendering functions from libass itself when it is ready and blend the resulting bitmaps immediately. This could be wrapped in a nice call like get_extended_osd_bitmaps(double pts). vf_ass is only added if vo does not support this.
I think this could be the clearest solution. Basically place the filter "inside the vo" if the vo supports that, otherwise place it in the normal filter chain.
supports the "extended" OSD (could also be used to check if there is a filter in between vf_ass and the vo that would break thinks, like the
This makes sense only if something between vf_ass and vo is messing with frames. I hope we can avoid this situation completely by adding vf_ass to the very end of the filter chain.
I think vf_ass should be usable as a general filter that can be placed anywhere in the chain when the vo special cases aren't used (not sure if you meant the "end of the filter chain" in the render-inside-vo case only - if so then this is of course irrelevant if there's no filter in the normal chain then).
On Monday 05 June 2006 04:47, Uoti Urpala wrote:
I think vf_ass should be usable as a general filter that can be placed anywhere in the chain when the vo special cases aren't used (not sure if you meant the "end of the filter chain" in the render-inside-vo case only - if so then this is of course irrelevant if there's no filter in the normal chain then).
I meant if we can't put the renderer inside the vo, lets put it just before the vo, as the last filter in the chain. This seems to be the most safe place for it. If user wants to put it in some other place - he probably knows what he is doing. In any case rendering and blending occur in the same place (either vf or vo), and bitmaps are never passed anywhere. Looks good.
Hi, On Mon, Jun 05, 2006 at 04:21:13AM +0400, Evgeniy Stepanov wrote:
They can store the pointer somewhere and use it later, when they get the whole frame. vf_ass can guarantee that it will be valid until the beginning of the next frame. [...] This makes sense only if something between vf_ass and vo is messing with frames. I hope we can avoid this situation completely by adding vf_ass to the very end of the filter chain.
And if there is something in between that causes more than one frame delay it will crash or even worse. I won't force anyone to do it perfect, I don't care enough about that, but it should be clear that this is horrible design, that will probably bite us sooner or later. Also, about an extra call in the vo: the vos themselves do not officially have the pts of the current frame, so I guess this would have to go into vf_vo - which probably is better anyway to avoid code duplication. Greetings, Reimar Döffinger
On Monday 05 June 2006 12:50, Reimar Döffinger wrote:
And if there is something in between that causes more than one frame delay it will crash or even worse. I won't force anyone to do it perfect, I don't care enough about that, but it should be clear that this is horrible design, that will probably bite us sooner or later.
Ok, it was a bad idea. I moved libass initialization and rendering back into vf_ass. VFCTRL_EOSD is used to determine who is going to render subtitles. The filter got "auto" option, which means "insert only if vo does not support EOSD". If -ass option is specified by user, and "ass" filter is not, then the filter is added automatically with "auto=1".
On Sun, 2006-06-11 at 20:15 +0400, Evgeniy Stepanov wrote:
I moved libass initialization and rendering back into vf_ass. VFCTRL_EOSD is
Seems you sent the wrong patch, the attached one is identical to the one from 2006-05-31.
On Sunday 11 June 2006 21:36, Uoti Urpala wrote:
Seems you sent the wrong patch, the attached one is identical to the one from 2006-05-31.
This is the right one.
Several bugfixes. Btw, since the previous patch all ass-specific stuff (rendering and processing) is done under "if (ass_enabled)". This means that not giving -ass option disables all possible bugs in my code (except for compilation errors, of course).
On 2006/06/14, at 5:09, Evgeniy Stepanov wrote:
Several bugfixes.
Btw, since the previous patch all ass-specific stuff (rendering and processing) is done under "if (ass_enabled)". This means that not giving -ass option disables all possible bugs in my code (except for compilation errors, of course).
This is my first time compiling mplayer with this and I am not a developer, but considering this isn't in the repository yet I thought I should post this bug report here. It fails to compile upon reaching vf_ass.c gcc version 3.3 libtool version 1.5 cc -c -I../libvo -I../../libvo -I/usr/X11R6/include -O4 -mcpu=7450 -mtune=7450 -pipe -ffast-math -fomit-frame-pointer -faltivec -D__APPLE_ALTIVEC__ -mdynamic-no-pic -falign-loops=16 -DSYS_DARWIN -DCONFIG_DARWIN -DCONFIG_DARWIN -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I. -I.. -Inative -I../libmpdemux -I../loader -I../libavutil -I../libavcodec -D_GNU_SOURCE -o vf_ass.o vf_ass.c In file included from vf_ass.c:452: vf.h:5: error: redefinition of `struct vf_info_s' vf.h:13: error: redefinition of `vf_info_t' vf.h:13: error: `vf_info_t' previously declared here vf.h:15: error: redefinition of `struct vf_image_context_s' vf.h:20: error: redefinition of `vf_image_context_t' vf.h:20: error: `vf_image_context_t' previously declared here vf.h:22: error: redefinition of `struct vf_format_context_t' vf.h:25: error: redefinition of `vf_format_context_t' vf.h:25: error: `vf_format_context_t' previously declared here vf.h:27: error: redefinition of `struct vf_instance_s' vf.h:56: error: redefinition of `vf_instance_t' vf.h:56: error: `vf_instance_t' previously declared here vf.h:62: error: redefinition of `struct vf_seteq_s' vf.h:65: error: redefinition of `vf_equalizer_t' vf.h:65: error: `vf_equalizer_t' previously declared here vf_ass.c:471: error: redefinition of `struct vf_priv_s' vf_ass.c:484: error: redefinition of `vf_priv_dflt' vf_ass.c:46: error: `vf_priv_dflt' previously defined here vf_ass.c:506: error: redefinition of `config' vf_ass.c:68: error: `config' previously defined here vf_ass.c:539: error: redefinition of `get_image' vf_ass.c:101: error: `get_image' previously defined here vf_ass.c:574: error: redefinition of `blank' vf_ass.c:136: error: `blank' previously defined here vf_ass.c:598: error: redefinition of `prepare_image' vf_ass.c:160: error: `prepare_image' previously defined here vf_ass.c:646: error: redefinition of `copy_from_image' vf_ass.c:208: error: `copy_from_image' previously defined here vf_ass.c:688: error: redefinition of `copy_to_image' vf_ass.c:250: error: `copy_to_image' previously defined here vf_ass.c:719: error: redefinition of `my_draw_bitmap' vf_ass.c:281: error: `my_draw_bitmap' previously defined here vf_ass.c:766: error: redefinition of `render_frame' vf_ass.c:328: error: `render_frame' previously defined here vf_ass.c:781: error: redefinition of `put_image' vf_ass.c:343: error: `put_image' previously defined here vf_ass.c:793: error: redefinition of `query_format' vf_ass.c:355: error: `query_format' previously defined here vf_ass.c:804: error: redefinition of `control' vf_ass.c:366: error: `control' previously defined here vf_ass.c:818: error: redefinition of `uninit' vf_ass.c:380: error: `uninit' previously defined here vf_ass.c:823: error: redefinition of `fmt_list' vf_ass.c:385: error: `fmt_list' previously defined here vf_ass.c:831: error: redefinition of `open' vf_ass.c:393: error: `open' previously defined here vf_ass.c:856: error: redefinition of `vf_opts_fields' vf_ass.c:418: error: `vf_opts_fields' previously defined here vf_ass.c:861: error: redefinition of `vf_opts' vf_ass.c:423: error: `vf_opts' previously defined here vf_ass.c:868: error: redefinition of `vf_info_ass' vf_ass.c:430: error: `vf_info_ass' previously defined here In file included from vf_ass.c:890: vf.h:5: error: redefinition of `struct vf_info_s' vf.h:13: error: redefinition of `vf_info_t' vf.h:13: error: `vf_info_t' previously declared here vf.h:15: error: redefinition of `struct vf_image_context_s' vf.h:20: error: redefinition of `vf_image_context_t' vf.h:20: error: `vf_image_context_t' previously declared here vf.h:22: error: redefinition of `struct vf_format_context_t' vf.h:25: error: redefinition of `vf_format_context_t' vf.h:25: error: `vf_format_context_t' previously declared here vf.h:27: error: redefinition of `struct vf_instance_s' vf.h:56: error: redefinition of `vf_instance_t' vf.h:56: error: `vf_instance_t' previously declared here vf.h:62: error: redefinition of `struct vf_seteq_s' vf.h:65: error: redefinition of `vf_equalizer_t' vf.h:65: error: `vf_equalizer_t' previously declared here vf_ass.c:909: error: redefinition of `struct vf_priv_s' vf_ass.c:922: error: redefinition of `vf_priv_dflt' vf_ass.c:484: error: `vf_priv_dflt' previously defined here vf_ass.c:944: error: redefinition of `config' vf_ass.c:506: error: `config' previously defined here vf_ass.c:977: error: redefinition of `get_image' vf_ass.c:539: error: `get_image' previously defined here vf_ass.c:1012: error: redefinition of `blank' vf_ass.c:574: error: `blank' previously defined here vf_ass.c:1036: error: redefinition of `prepare_image' vf_ass.c:598: error: `prepare_image' previously defined here vf_ass.c:1084: error: redefinition of `copy_from_image' vf_ass.c:646: error: `copy_from_image' previously defined here vf_ass.c:1126: error: redefinition of `copy_to_image' vf_ass.c:688: error: `copy_to_image' previously defined here vf_ass.c:1157: error: redefinition of `my_draw_bitmap' vf_ass.c:719: error: `my_draw_bitmap' previously defined here vf_ass.c:1204: error: redefinition of `render_frame' vf_ass.c:766: error: `render_frame' previously defined here vf_ass.c:1219: error: redefinition of `put_image' vf_ass.c:781: error: `put_image' previously defined here vf_ass.c:1231: error: redefinition of `query_format' vf_ass.c:793: error: `query_format' previously defined here vf_ass.c:1242: error: redefinition of `control' vf_ass.c:804: error: `control' previously defined here vf_ass.c:1256: error: redefinition of `uninit' vf_ass.c:818: error: `uninit' previously defined here vf_ass.c:1261: error: redefinition of `fmt_list' vf_ass.c:823: error: `fmt_list' previously defined here vf_ass.c:1269: error: redefinition of `open' vf_ass.c:831: error: `open' previously defined here vf_ass.c:1294: error: redefinition of `vf_opts_fields' vf_ass.c:856: error: `vf_opts_fields' previously defined here vf_ass.c:1299: error: redefinition of `vf_opts' vf_ass.c:861: error: `vf_opts' previously defined here vf_ass.c:1306: error: redefinition of `vf_info_ass' vf_ass.c:868: error: `vf_info_ass' previously defined here In file included from vf_ass.c:1328: vf.h:5: error: redefinition of `struct vf_info_s' vf.h:13: error: redefinition of `vf_info_t' vf.h:13: error: `vf_info_t' previously declared here vf.h:15: error: redefinition of `struct vf_image_context_s' vf.h:20: error: redefinition of `vf_image_context_t' vf.h:20: error: `vf_image_context_t' previously declared here vf.h:22: error: redefinition of `struct vf_format_context_t' vf.h:25: error: redefinition of `vf_format_context_t' vf.h:25: error: `vf_format_context_t' previously declared here vf.h:27: error: redefinition of `struct vf_instance_s' vf.h:56: error: redefinition of `vf_instance_t' vf.h:56: error: `vf_instance_t' previously declared here vf.h:62: error: redefinition of `struct vf_seteq_s' vf.h:65: error: redefinition of `vf_equalizer_t' vf.h:65: error: `vf_equalizer_t' previously declared here vf_ass.c:1347: error: redefinition of `struct vf_priv_s' vf_ass.c:1360: error: redefinition of `vf_priv_dflt' vf_ass.c:922: error: `vf_priv_dflt' previously defined here vf_ass.c:1382: error: redefinition of `config' vf_ass.c:944: error: `config' previously defined here vf_ass.c:1415: error: redefinition of `get_image' vf_ass.c:977: error: `get_image' previously defined here vf_ass.c:1450: error: redefinition of `blank' vf_ass.c:1012: error: `blank' previously defined here vf_ass.c:1474: error: redefinition of `prepare_image' vf_ass.c:1036: error: `prepare_image' previously defined here vf_ass.c:1522: error: redefinition of `copy_from_image' vf_ass.c:1084: error: `copy_from_image' previously defined here vf_ass.c:1564: error: redefinition of `copy_to_image' vf_ass.c:1126: error: `copy_to_image' previously defined here vf_ass.c:1595: error: redefinition of `my_draw_bitmap' vf_ass.c:1157: error: `my_draw_bitmap' previously defined here vf_ass.c:1642: error: redefinition of `render_frame' vf_ass.c:1204: error: `render_frame' previously defined here vf_ass.c:1657: error: redefinition of `put_image' vf_ass.c:1219: error: `put_image' previously defined here vf_ass.c:1669: error: redefinition of `query_format' vf_ass.c:1231: error: `query_format' previously defined here vf_ass.c:1680: error: redefinition of `control' vf_ass.c:1242: error: `control' previously defined here vf_ass.c:1694: error: redefinition of `uninit' vf_ass.c:1256: error: `uninit' previously defined here vf_ass.c:1699: error: redefinition of `fmt_list' vf_ass.c:1261: error: `fmt_list' previously defined here vf_ass.c:1707: error: redefinition of `open' vf_ass.c:1269: error: `open' previously defined here vf_ass.c:1732: error: redefinition of `vf_opts_fields' vf_ass.c:1294: error: `vf_opts_fields' previously defined here vf_ass.c:1737: error: redefinition of `vf_opts' vf_ass.c:1299: error: `vf_opts' previously defined here vf_ass.c:1744: error: redefinition of `vf_info_ass' vf_ass.c:1306: error: `vf_info_ass' previously defined here In file included from vf_ass.c:1766: vf.h:5: error: redefinition of `struct vf_info_s' vf.h:13: error: redefinition of `vf_info_t' vf.h:13: error: `vf_info_t' previously declared here vf.h:15: error: redefinition of `struct vf_image_context_s' vf.h:20: error: redefinition of `vf_image_context_t' vf.h:20: error: `vf_image_context_t' previously declared here vf.h:22: error: redefinition of `struct vf_format_context_t' vf.h:25: error: redefinition of `vf_format_context_t' vf.h:25: error: `vf_format_context_t' previously declared here vf.h:27: error: redefinition of `struct vf_instance_s' vf.h:56: error: redefinition of `vf_instance_t' vf.h:56: error: `vf_instance_t' previously declared here vf.h:62: error: redefinition of `struct vf_seteq_s' vf.h:65: error: redefinition of `vf_equalizer_t' vf.h:65: error: `vf_equalizer_t' previously declared here vf_ass.c:1785: error: redefinition of `struct vf_priv_s' vf_ass.c:1798: error: redefinition of `vf_priv_dflt' vf_ass.c:1360: error: `vf_priv_dflt' previously defined here vf_ass.c:1820: error: redefinition of `config' vf_ass.c:1382: error: `config' previously defined here vf_ass.c:1853: error: redefinition of `get_image' vf_ass.c:1415: error: `get_image' previously defined here vf_ass.c:1888: error: redefinition of `blank' vf_ass.c:1450: error: `blank' previously defined here vf_ass.c:1912: error: redefinition of `prepare_image' vf_ass.c:1474: error: `prepare_image' previously defined here vf_ass.c:1960: error: redefinition of `copy_from_image' vf_ass.c:1522: error: `copy_from_image' previously defined here vf_ass.c:2002: error: redefinition of `copy_to_image' vf_ass.c:1564: error: `copy_to_image' previously defined here vf_ass.c:2033: error: redefinition of `my_draw_bitmap' vf_ass.c:1595: error: `my_draw_bitmap' previously defined here vf_ass.c:2080: error: redefinition of `render_frame' vf_ass.c:1642: error: `render_frame' previously defined here vf_ass.c:2095: error: redefinition of `put_image' vf_ass.c:1657: error: `put_image' previously defined here vf_ass.c:2107: error: redefinition of `query_format' vf_ass.c:1669: error: `query_format' previously defined here vf_ass.c:2118: error: redefinition of `control' vf_ass.c:1680: error: `control' previously defined here vf_ass.c:2132: error: redefinition of `uninit' vf_ass.c:1694: error: `uninit' previously defined here vf_ass.c:2137: error: redefinition of `fmt_list' vf_ass.c:1699: error: `fmt_list' previously defined here vf_ass.c:2145: error: redefinition of `open' vf_ass.c:1707: error: `open' previously defined here vf_ass.c:2170: error: redefinition of `vf_opts_fields' vf_ass.c:1732: error: `vf_opts_fields' previously defined here vf_ass.c:2175: error: redefinition of `vf_opts' vf_ass.c:1737: error: `vf_opts' previously defined here vf_ass.c:2182: error: redefinition of `vf_info_ass' vf_ass.c:1744: error: `vf_info_ass' previously defined here In file included from vf_ass.c:2204: vf.h:5: error: redefinition of `struct vf_info_s' vf.h:13: error: redefinition of `vf_info_t' vf.h:13: error: `vf_info_t' previously declared here vf.h:15: error: redefinition of `struct vf_image_context_s' vf.h:20: error: redefinition of `vf_image_context_t' vf.h:20: error: `vf_image_context_t' previously declared here vf.h:22: error: redefinition of `struct vf_format_context_t' vf.h:25: error: redefinition of `vf_format_context_t' vf.h:25: error: `vf_format_context_t' previously declared here vf.h:27: error: redefinition of `struct vf_instance_s' vf.h:56: error: redefinition of `vf_instance_t' vf.h:56: error: `vf_instance_t' previously declared here vf.h:62: error: redefinition of `struct vf_seteq_s' vf.h:65: error: redefinition of `vf_equalizer_t' vf.h:65: error: `vf_equalizer_t' previously declared here vf_ass.c:2223: error: redefinition of `struct vf_priv_s' vf_ass.c:2236: error: redefinition of `vf_priv_dflt' vf_ass.c:1798: error: `vf_priv_dflt' previously defined here vf_ass.c:2258: error: redefinition of `config' vf_ass.c:1820: error: `config' previously defined here vf_ass.c:2291: error: redefinition of `get_image' vf_ass.c:1853: error: `get_image' previously defined here vf_ass.c:2326: error: redefinition of `blank' vf_ass.c:1888: error: `blank' previously defined here vf_ass.c:2350: error: redefinition of `prepare_image' vf_ass.c:1912: error: `prepare_image' previously defined here vf_ass.c:2398: error: redefinition of `copy_from_image' vf_ass.c:1960: error: `copy_from_image' previously defined here vf_ass.c:2440: error: redefinition of `copy_to_image' vf_ass.c:2002: error: `copy_to_image' previously defined here vf_ass.c:2471: error: redefinition of `my_draw_bitmap' vf_ass.c:2033: error: `my_draw_bitmap' previously defined here vf_ass.c:2518: error: redefinition of `render_frame' vf_ass.c:2080: error: `render_frame' previously defined here vf_ass.c:2533: error: redefinition of `put_image' vf_ass.c:2095: error: `put_image' previously defined here vf_ass.c:2545: error: redefinition of `query_format' vf_ass.c:2107: error: `query_format' previously defined here vf_ass.c:2556: error: redefinition of `control' vf_ass.c:2118: error: `control' previously defined here vf_ass.c:2570: error: redefinition of `uninit' vf_ass.c:2132: error: `uninit' previously defined here vf_ass.c:2575: error: redefinition of `fmt_list' vf_ass.c:2137: error: `fmt_list' previously defined here vf_ass.c:2583: error: redefinition of `open' vf_ass.c:2145: error: `open' previously defined here vf_ass.c:2608: error: redefinition of `vf_opts_fields' vf_ass.c:2170: error: `vf_opts_fields' previously defined here vf_ass.c:2613: error: redefinition of `vf_opts' vf_ass.c:2175: error: `vf_opts' previously defined here vf_ass.c:2620: error: redefinition of `vf_info_ass' vf_ass.c:2182: error: `vf_info_ass' previously defined here {standard input}:74:Ignoring attempt to re-define symbol. {standard input}:76:FATAL:Symbol _fmt_list already defined. make[1]: *** [vf_ass.o] Error 1 make: *** [libmpcodecs/libmpcodecs.a] Error 2
On Thu, 2006-06-15 at 07:08 -0400, Ergzay wrote:
cc -c -I../libvo -I../../libvo -I/usr/X11R6/include -O4 -mcpu=7450 -mtune=7450 -pipe -ffast-math -fomit-frame-pointer -faltivec -D__APPLE_ALTIVEC__ -mdynamic-no-pic -falign-loops=16 -DSYS_DARWIN -DCONFIG_DARWIN -DCONFIG_DARWIN -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I. -I.. -Inative -I../libmpdemux -I../loader -I../libavutil -I../libavcodec -D_GNU_SOURCE -o vf_ass.o vf_ass.c In file included from vf_ass.c:452: vf.h:5: error: redefinition of `struct vf_info_s'
You must have broken your local source tree somehow, that error doesn't match the contents of the patch. vf_ass.c doesn't even have 452 lines, and the last include is on line 22. Maybe you somehow applied the patch twice resulting in two copies of the file concatenated together?
Hello, On Wed, Jun 14, 2006 at 01:09:42PM +0400, Evgeniy Stepanov wrote:
Several bugfixes.
Some reviewing and a question. Maybe even those developers without the time to review themselves can review my review...
Index: libmpcodecs/vf.h =================================================================== --- libmpcodecs/vf.h (.../branches/upstream) (revision 503) +++ libmpcodecs/vf.h (.../trunk) (revision 503) @@ -75,6 +75,7 @@ #define VFCTRL_SKIP_NEXT_FRAME 12 /* For encoding - drop the next frame that passes thru */ #define VFCTRL_FLUSH_FRAMES 13 /* For encoding - flush delayed frames */ #define VFCTRL_SCREENSHOT 14 /* Make a screenshot */ +#define VFCTRL_EOSD 15 /* Select EOSD renderer */
The other values are aligned so they are in the same column, so it should be done here, too in configure:
@@ -5387,6 +5391,35 @@ fi echores "$_fontconfig"
+echocheck "SSA/ASS support" +# libass depends on freetype +if test "$_freetype" = no ; then + _ass=no + _res_comment="FreeType support needed" +fi
IMHO this should go in the "auto" case, --enable should always be respected, regardless of the consequences. in mplayer.c:
@@ -1845,6 +1861,20 @@ return 1; } #endif +#ifdef USE_ASS + if (ass_track && ass_track->name) { + char *tmp,*tmp2; + tmp = ass_track->name; + if ((tmp2 = strrchr(tmp, '/'))) + tmp = tmp2+1; + + snprintf(*(char**)arg, 63, "(%d) %s%s", + set_of_sub_pos + 1, + strlen(tmp) < 20 ? "" : "...", + strlen(tmp) < 20 ? tmp : tmp+strlen(tmp)-19); + return 1; + } +#endif
Please do not duplicate code. Either move it in a seperate function or just use the existing code e.g. by changing "if (subdata)" to "if (subdata || ass_track)" and "tmp = subdata->filename;" to "tmp = subdata ? subdata->filename : ass_track->name;" You can check for tmp == NULL then and set some useful substitute...
@@ -1925,15 +1955,25 @@ [...] - subdata = set_of_subtitles[set_of_sub_pos]; - vo_osd_changed(OSDTYPE_SUBTITLE); +#ifdef USE_ASS + if (ass_enabled && set_of_ass_tracks[set_of_sub_pos]) + ass_track = set_of_ass_tracks[set_of_sub_pos]; + else #endif + { + subdata = set_of_subtitles[set_of_sub_pos]; + vo_osd_changed(OSDTYPE_SUBTITLE);
For improved patch-readability, I'd say don't change then indentation of these two lines.
@@ -3459,11 +3505,37 @@ [...] +#ifdef USE_ASS +if (ass_enabled) + ((vf_instance_t *)sh_video->vfilter)->control(sh_video->vfilter, VFCTRL_EOSD, 0); +#endif +
What is the intended purpose of this?
@@ -5084,9 +5160,15 @@ current_module="sub_free"; for (i = 0; i < set_of_sub_size; ++i) sub_free( set_of_subtitles[i] ); - set_of_sub_size = 0; vo_sub_last = vo_sub=NULL; subdata=NULL; +#ifdef USE_ASS + for (i = 0; i < set_of_sub_size; ++i) + if ( set_of_ass_tracks[i] ) + ass_free_track( set_of_ass_tracks[i] ); + ass_track = NULL; +#endif + set_of_sub_size = 0;
Why not save some code and do both frees in the same loop?
Index: libass/ass_mp.c =================================================================== --- libass/ass_mp.c (.../branches/upstream) (revision 0) +++ libass/ass_mp.c (.../trunk) (revision 503) @@ -0,0 +1,9 @@ +#include "ass_mp.h" + +// libass-related command line options +float ass_font_scale = 1.; +float ass_line_spacing = 0.; +int ass_top_margin = 0; +int ass_bottom_margin = 0; +int extract_embedded_fonts = 0; +
A whole file for just nine lines seems a bit like overkill to me, but your decision. in ass_render.c:
+#define PI 3.1415926535897932384626433832795029L
why not just use M_PI? I think we already (e.g. in the audio filters) require that to be defined properly in math.h
+#define _r(c) ((c)>>24) +#define _g(c) (((c)>>16)&0xFF) +#define _b(c) (((c)>>8)&0xFF) +#define _a(c) ((c)&0xFF) +#define rgba2y(c) ( (( 263*_r(c) + 516*_g(c) + 100*_b(c)) >> 10) + 16 ) +#define rgba2u(c) ( ((-152*_r(c) - 298*_g(c) + 450*_b(c)) >> 10) + 128 ) +#define rgba2v(c) ( (( 450*_r(c) - 376*_g(c) - 73*_b(c)) >> 10) + 128 )
These seem to be unused??
+/** + * \brief Parse color value from ass script. + */ +static inline unsigned ampcolor(char* p, char** q) { +#define badcolor { mp_msg(MSGT_GLOBAL, MSGL_WARN, "bad color at %s:%d\n", __FILE__, __LINE__); *q = p; return 0; }
IMHO this is ugly and also bloats code. Normally I woud suggest using goto, but here:
+ unsigned color = 0; + if (*p++ != '&') badcolor + if (*p++ != 'H') badcolor
why not just if (*p++ != '&' || *p++ != 'H') { mp_msg(...); } Ok, you can't make a difference between the '&' and 'H' expected cases then...
+/** + * \brief Calculate a weighted average of two colors + * calculates c1*(1-a) + c2*a, but separately for each component + */ +static unsigned color_avg(unsigned c1, unsigned c2, double a) +{ + unsigned char* p1 = (unsigned char*)&c1; + unsigned char* p2 = (unsigned char*)&c2; + unsigned c3; + unsigned char* p3 = (unsigned char*)&c3; + int i; + for (i = 0; i < 4; ++i) + p3[i] = p1[i] * (1 - a) + p2[i] * a; + return c3;
If something absolutely needs to have 4 bytes size, please use uint32_t instead of just unsigned. Also
+static void change_alpha(unsigned* var, unsigned new_val, double pwr) +{ + (*var) = color_avg(*var, ((*var) & 0xFFFFFF00) | new_val, pwr); +} + +static void change_color(unsigned* var, unsigned new_val, double pwr) +{ + (*var) = color_avg(*var, ((*var) & 0x000000FF) | new_val, pwr); +}
Interpolating all four component when only one respectively three values are different is a bit overkill... Maybe it would be simpler to make c1, c2 and all internal color stuff uin8_t c1[4] instead of unsigned and convert via the usual (c1[0] << 24) | (c1[1] << 16) | (c1[2] << 8) | c1[3] (see the BE_32 macro in libavcodec), though you might have to be more careful to make sure it works with any endianness.
+static void skip_spaces(char** str) { + char* p = *str; + while ((*p==' ') || (*p=='\t')) + ++p; + *str = p; +}
Is this what the specification defines as valid spaces? I think a comment on this function would be nice, since e.g. UTF-8 sure specifies a lot more as space...
+static unsigned string2color(char* p) { +#define badcolor { mp_msg(MSGT_GLOBAL, MSGL_WARN, "bad color at %s:%d\n", __FILE__, __LINE__); return 0; }
Huh? What is the difference between this function and the previous one where I complained about this macro?
+#define NEXT(str,token) \ + token = next_token(&str); \ + if (!token) break; + +#define ANYVAL(name,func) \ + } else if (strcasecmp(tname, #name) == 0) { \ + target->name = func(token); \ + mp_msg(MSGT_GLOBAL, MSGL_DBG2, "%s = %s\n", #name, token); +#define STRVAL(name) ANYVAL(name,strdup) +#define COLORVAL(name) ANYVAL(name,string2color) +#define INTVAL(name) ANYVAL(name,atoi) +#define FPVAL(name) ANYVAL(name,atof) +#define TIMEVAL(name) ANYVAL(name,string2timecode) +#define STYLEVAL(name) \ + } else if (strcasecmp(tname, #name) == 0) { \ + target->name = lookup_style(track, token); \ + mp_msg(MSGT_GLOBAL, MSGL_DBG2, "%s = %s\n", #name, token); + +#define ALIAS(alias,name) \ + if (strcasecmp(tname, #alias) == 0) {tname = #name;}
Hmmm... maybe you can convert some of these into functions, I think it might help readability a bit (esp. when it comes to checking for possible vulnerabilities - which I admit I am not doing).
+static void* guess_cp(char* data, int size, char *preferred_language, char *fallback)
How does this function relate to the code in subreader.c? Can that one be change to use this function, too?
+static char* validate_fname(char* name)
I am not sure this is careful enough - though I think this feature is a problem in itself.
+/** + * \brief Process embedded matroska font. Saves it to ~/.mplayer/fonts.
I do not like this at all. This feature might be misused in many ways, one I can think of is leaving a track record of which films somebody has been watching. The fact that it is disabled by default might make it acceptable though. I prefer if there was a way to use embedded fonts with at most using temporary files created via tmpfile instead (hopefully this function is portable enough).
Index: libass/ass_utils.c =================================================================== --- libass/ass_utils.c (.../branches/upstream) (revision 0) +++ libass/ass_utils.c (.../trunk) (revision 503) @@ -0,0 +1,39 @@ +#include "config.h" + +#include <stdint.h> +#include <sys/time.h> +#include <time.h> + +#include "mp_msg.h" +#include "ass_utils.h" + +unsigned utf8_to_ucs4(char** str)
I give whoever commits the wonderful duty to find a place for this function and the utf8_get_char in libvo/sub.c (they are the same).
+#define MAX_FACE_CACHE_SIZE 100 + +static face_cache_item_t face_cache[MAX_FACE_CACHE_SIZE];
How big is this? Might be better to allocate at runtime because if I understand things correctly this will increase startup time and memory consumption even if ass is not used at all...
+static unsigned glyph_hash(glyph_hash_key_t* key) { + unsigned val = 0; + unsigned i; + for (i = 0; i < sizeof(key->face); ++i) + val += *(unsigned char *)(&(key->face) + i); + val <<= 21;
This does not look like the keys will be very unique. Can this be a problem? Somebody please make Moritz review the changes to demux_mkv *g*
Index: mencoder.c =================================================================== --- mencoder.c (.../branches/upstream) (revision 503) +++ mencoder.c (.../trunk) (revision 503) @@ -54,6 +54,7 @@ #include "libmpdemux/stheader.h" #include "libmpdemux/mp3_hdr.h" #include "libmpdemux/muxer.h" +#include "libmpdemux/matroska.h"
Why is this needed? Can it be avoided? (if this is in mplayer.c as well, same question there *g*) And now for something completely different. What do you think would be needed to support this in vo_gl.c? What would be the steps needed to get one or several bitmaps containing the overlay data for the current frame? Greetings, Reimar Döffinger P.S. I did not review any of the parts under #if 0, might be a good idea to remove them before committing.
On Saturday 17 June 2006 23:27, Reimar Döffinger wrote:
Some reviewing and a question. Maybe even those developers without the time to review themselves can review my review...
Index: libmpcodecs/vf.h =================================================================== --- libmpcodecs/vf.h (.../branches/upstream) (revision 503) +++ libmpcodecs/vf.h (.../trunk) (revision 503) @@ -75,6 +75,7 @@ #define VFCTRL_SKIP_NEXT_FRAME 12 /* For encoding - drop the next frame that passes thru */ #define VFCTRL_FLUSH_FRAMES 13 /* For encoding - flush delayed frames */ #define VFCTRL_SCREENSHOT 14 /* Make a screenshot */ +#define VFCTRL_EOSD 15 /* Select EOSD renderer */
The other values are aligned so they are in the same column, so it should be done here, too
You mean on the same column as the last four of them ? No problem :)
in configure:
@@ -5387,6 +5391,35 @@ fi echores "$_fontconfig"
+echocheck "SSA/ASS support" +# libass depends on freetype +if test "$_freetype" = no ; then + _ass=no + _res_comment="FreeType support needed" +fi
IMHO this should go in the "auto" case, --enable should always be respected, regardless of the consequences.
FreeType and FontConfig checks just above do exactly the same thing. If you are right, they should also be fixed to avoid inconsistency.
@@ -1925,15 +1955,25 @@
[...]
- subdata = set_of_subtitles[set_of_sub_pos]; - vo_osd_changed(OSDTYPE_SUBTITLE); +#ifdef USE_ASS + if (ass_enabled && set_of_ass_tracks[set_of_sub_pos]) + ass_track = set_of_ass_tracks[set_of_sub_pos]; + else #endif + { + subdata = set_of_subtitles[set_of_sub_pos]; + vo_osd_changed(OSDTYPE_SUBTITLE);
For improved patch-readability, I'd say don't change then indentation of these two lines.
Even though they have been moved under if() {} ? That's a tradeoff between patch readability and code readability :) I prefer the latter.
@@ -3459,11 +3505,37 @@
[...]
+#ifdef USE_ASS +if (ass_enabled) + ((vf_instance_t *)sh_video->vfilter)->control(sh_video->vfilter, VFCTRL_EOSD, 0); +#endif +
What is the intended purpose of this?
This is to select who will display subtitles - either filter or vo. We can have both vf_ass and eosd-capable vo at the same time, if user explicitly asked for it (with "-ass -vf ass"). In this case the vo will not be used.
Index: libass/ass_mp.c =================================================================== --- libass/ass_mp.c (.../branches/upstream) (revision 0) +++ libass/ass_mp.c (.../trunk) (revision 503) @@ -0,0 +1,9 @@ +#include "ass_mp.h" + +// libass-related command line options +float ass_font_scale = 1.; +float ass_line_spacing = 0.; +int ass_top_margin = 0; +int ass_bottom_margin = 0; +int extract_embedded_fonts = 0; +
A whole file for just nine lines seems a bit like overkill to me, but your decision.
This file is intended for mplayer-specific things. Currently there are only configuration variables, which i otherwise would have to place in both mplayer.c and mencoder.c. Soon I'm going to add subdata -> ass_track_t conversion function here and achieve the dream of colored srt subtitles.
+static void skip_spaces(char** str) { + char* p = *str; + while ((*p==' ') || (*p=='\t')) + ++p; + *str = p; +}
Is this what the specification defines as valid spaces? I think a comment on this function would be nice, since e.g. UTF-8 sure specifies a lot more as space...
Specification says nothing about spaces, and this function works with all scripts I've seen. I'll fix it someday.
+static void* guess_cp(char* data, int size, char *preferred_language, char *fallback)
How does this function relate to the code in subreader.c? Can that one be change to use this function, too?
They are almost the same except for stream_t stuff. That's possible, but this one should be moved out from #ifdef USE_ASS then.
+static char* validate_fname(char* name)
I am not sure this is careful enough - though I think this feature is a problem in itself.
I'm not sure either. I've found this list of chars somewhere in MSDN. Probably it would be better to only allow some subset of ascii (letters, digits, so on).
+/** + * \brief Process embedded matroska font. Saves it to ~/.mplayer/fonts.
I do not like this at all. This feature might be misused in many ways, one I can think of is leaving a track record of which films somebody has been watching. The fact that it is disabled by default might make it acceptable though. I prefer if there was a way to use embedded fonts with at most using temporary files created via tmpfile instead (hopefully this function is portable enough).
Freetype can read fonts directly from memory, but fontconfig can't :( I like to have a cache of fonts. What about creating persistent files with -embeddedfonts and temporary ones otherwise ? Do we need a mode when no files will be created at all ?
+#define MAX_FACE_CACHE_SIZE 100 + +static face_cache_item_t face_cache[MAX_FACE_CACHE_SIZE];
How big is this? Might be better to allocate at runtime because if I understand things correctly this will increase startup time and memory consumption even if ass is not used at all...
As far as I know, this will only consume some virtual address space. Even so, dynamic allocation is a good idea.
+static unsigned glyph_hash(glyph_hash_key_t* key) { + unsigned val = 0; + unsigned i; + for (i = 0; i < sizeof(key->face); ++i) + val += *(unsigned char *)(&(key->face) + i); + val <<= 21;
This does not look like the keys will be very unique. Can this be a problem?
Usually there are few different font faces used, so this should be ok.
Index: mencoder.c =================================================================== --- mencoder.c (.../branches/upstream) (revision 503) +++ mencoder.c (.../trunk) (revision 503) @@ -54,6 +54,7 @@ #include "libmpdemux/stheader.h" #include "libmpdemux/mp3_hdr.h" #include "libmpdemux/muxer.h" +#include "libmpdemux/matroska.h"
Why is this needed? Can it be avoided? (if this is in mplayer.c as well, same question there *g*)
For mkv_sh_sub_t definition. I agree with everything else you mentioned, and will fix it in a few days.
On Sun, 2006-06-18 at 02:33 +0400, Evgeniy Stepanov wrote:
On Saturday 17 June 2006 23:27, Reimar Döffinger wrote:
+if test "$_freetype" = no ; then + _ass=no + _res_comment="FreeType support needed" +fi
IMHO this should go in the "auto" case, --enable should always be respected, regardless of the consequences.
FreeType and FontConfig checks just above do exactly the same thing. If you are right, they should also be fixed to avoid inconsistency.
I think this does not need to go in the "auto" case. This isn't about checking whether the library is available but about whether it's internally enabled; the user should --enable freetype too if he wants to force it.
Even though they have been moved under if() {} ? That's a tradeoff between patch readability and code readability :) I prefer the latter.
I also prefer having such indentation changes in the same patch in this case.
This file is intended for mplayer-specific things. Currently there are only configuration variables, which i otherwise would have to place in both mplayer.c and mencoder.c. Soon I'm going to add subdata -> ass_track_t conversion function here and achieve the dream of colored srt subtitles.
Not directly related to above case, but I think it might be better add support to mencoder separately later. Also adding the timestamps to the filter chain is not currently safe for mencoder and does break things (mencoder is quite broken in several ways and would probably need a large rewrite).
+static char* validate_fname(char* name)
I am not sure this is careful enough - though I think this feature is a problem in itself.
I'm not sure either. I've found this list of chars somewhere in MSDN. Probably it would be better to only allow some subset of ascii (letters, digits, so on).
The current version seems questionable in a couple of ways. It replaces "unsafe" characters with '_', but the resulting filename isn't stored anywhere and it seems that such files are unlikely to ever be successfully loaded by anything after they've been saved on disk (fontconfig won't find fonts after the name has been changed etc), so you could just as well skip saving them. The utf-8 decoding also seems unnecessary and could allow bytes with high bit set in filenames even though the code now tries to disallow that (the decoding doesn't check for nonstandard longer-than-normal representations so you could encode a small character with a long utf-8 sequence having high bits set).
Freetype can read fonts directly from memory, but fontconfig can't :( I like to have a cache of fonts. What about creating persistent files with -embeddedfonts and temporary ones otherwise ? Do we need a mode when no files will be created at all ?
There does need to be a mode that doesn't require writing on disk. Having a temporary-file mode might be desirable but IMO it's not a feature that would be necessary to have before committing the patch to MPlayer svn.
On Sunday 18 June 2006 03:17, Uoti Urpala wrote:
The current version seems questionable in a couple of ways. It replaces "unsafe" characters with '_', but the resulting filename isn't stored anywhere and it seems that such files are unlikely to ever be successfully loaded by anything after they've been saved on disk (fontconfig won't find fonts after the name has been changed etc), so you could just as well skip saving them. The utf-8 decoding also seems unnecessary and could allow bytes with high bit set in filenames even though the code now tries to disallow that (the decoding doesn't check for nonstandard longer-than-normal representations so you could encode a small character with a long utf-8 sequence having high bits set).
The purpose of this code is to generate a valid file name most closely resembling the original one. Just for humans. Fontconfig matches fonts based on their properties like family and weight, ignoring the file name. Maybe it would be better to name them like 1.ttf, 2.ttf. After all, mplayer is not a tool for attachment extraction. In that case there is no need to keep old fonts, and we can use temporary files. No need for this portable filename validation code.
On Sun, 2006-06-18 at 04:03 +0400, Evgeniy Stepanov wrote:
The purpose of this code is to generate a valid file name most closely resembling the original one. Just for humans. Fontconfig matches fonts based on their properties like family and weight, ignoring the file name.
Yes seems it reads all files in the directory when starting. That might be a reason not to have a big cache of unindexed font files as loading them all on startup could cause a noticeable delay.
Maybe it would be better to name them like 1.ttf, 2.ttf. After all, mplayer is not a tool for attachment extraction. In that case there is no need to keep old fonts, and we can use temporary files. No need for this portable filename
I don't understand why you say "In that case there is no need to keep old fonts" - how is the naming scheme for the files relevant to whether you want to keep them?
On Sunday 18 June 2006 04:32, Uoti Urpala wrote:
I don't understand why you say "In that case there is no need to keep old fonts" - how is the naming scheme for the files relevant to whether you want to keep them?
I meant, they won't be needed for playback (probably), because all the needed files are usualy included in container. So, the only reason to keep them is for the user. He may want to keep a font he likes, or something like that. We can have three modes: -noembeddedfonts -embeddedfonts -keepembeddedfonts
On Sunday, 18 June 2006 at 12:49, Evgeniy Stepanov wrote:
On Sunday 18 June 2006 04:32, Uoti Urpala wrote:
I don't understand why you say "In that case there is no need to keep old fonts" - how is the naming scheme for the files relevant to whether you want to keep them?
I meant, they won't be needed for playback (probably), because all the needed files are usualy included in container. So, the only reason to keep them is for the user. He may want to keep a font he likes, or something like that.
We can have three modes: -noembeddedfonts -embeddedfonts -keepembeddedfonts
How about -embeddedfonts=keep instead of the last one? R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Sun, Jun 18, 2006 at 02:33:13AM +0400, Evgeniy Stepanov wrote:
@@ -1925,15 +1955,25 @@
[...]
- subdata = set_of_subtitles[set_of_sub_pos]; - vo_osd_changed(OSDTYPE_SUBTITLE); +#ifdef USE_ASS + if (ass_enabled && set_of_ass_tracks[set_of_sub_pos]) + ass_track = set_of_ass_tracks[set_of_sub_pos]; + else #endif + { + subdata = set_of_subtitles[set_of_sub_pos]; + vo_osd_changed(OSDTYPE_SUBTITLE);
For improved patch-readability, I'd say don't change then indentation of these two lines.
Even though they have been moved under if() {} ? That's a tradeoff between patch readability and code readability :) I prefer the latter.
You can do the cosmetics in a separate subsequent patch. Mixing them makes it hard to review and hard for people going back later to audit.
+/** + * \brief Process embedded matroska font. Saves it to ~/.mplayer/fonts.
I do not like this at all. This feature might be misused in many ways, one I can think of is leaving a track record of which films somebody has been watching. The fact that it is disabled by default might make it acceptable though. I prefer if there was a way to use embedded fonts with at most using temporary files created via tmpfile instead (hopefully this function is portable enough).
Freetype can read fonts directly from memory, but fontconfig can't :(
Is there a reason we need fontconfig? I hate it...
I like to have a cache of fonts. What about creating persistent files with -embeddedfonts and temporary ones otherwise ? Do we need a mode when no files will be created at all ?
Yes absolutely. Creating files is a very very bad thing. It will not work on read-only filesystems (think live cd with MPlayer and movie files) and it leads to bugs and security issues more often than not. Why not just bypass fontconfig? I don't even have fontconfig and I'm quite happy with the way MPlayer works.
+#define MAX_FACE_CACHE_SIZE 100 + +static face_cache_item_t face_cache[MAX_FACE_CACHE_SIZE];
How big is this? Might be better to allocate at runtime because if I understand things correctly this will increase startup time and memory consumption even if ass is not used at all...
As far as I know, this will only consume some virtual address space. Even so, dynamic allocation is a good idea.
Unless it's absolutely huge, this will translate to physical space, since it will put more empty space within whatever page it ends up in, and this page will still be dirty (written) due to other variables in the same page. Yes individually these things are small but MPlayer is a huge app and they will surely add up once there are lots of similarly bad designs.
Index: mencoder.c =================================================================== --- mencoder.c (.../branches/upstream) (revision 503) +++ mencoder.c (.../trunk) (revision 503) @@ -54,6 +54,7 @@ #include "libmpdemux/stheader.h" #include "libmpdemux/mp3_hdr.h" #include "libmpdemux/muxer.h" +#include "libmpdemux/matroska.h"
Why is this needed? Can it be avoided? (if this is in mplayer.c as well, same question there *g*)
For mkv_sh_sub_t definition.
Should be some way to fix this to be demuxer-independent. Rich
On Sun, 2006-06-18 at 09:38 -0400, Rich Felker wrote:
Even though they have been moved under if() {} ? That's a tradeoff between patch readability and code readability :) I prefer the latter.
You can do the cosmetics in a separate subsequent patch. Mixing them makes it hard to review and hard for people going back later to audit.
That was not an isolated cosmetic change but an indentation change related to new functionality. I think it's better to keep those as part of the same patch as long as they don't reindent a block of several dozen lines.
Freetype can read fonts directly from memory, but fontconfig can't :(
Is there a reason we need fontconfig? I hate it...
Is there a reason you hate fontconfig? You hate various things, often for completely irrational reasons.
Why not just bypass fontconfig? I don't even have fontconfig and I'm quite happy with the way MPlayer works.
Whether you're happy with the way MPlayer currently works with files that do not specify fonts is pretty much irrelevant.
On Sun, Jun 18, 2006 at 04:59:50PM +0300, Uoti Urpala wrote:
On Sun, 2006-06-18 at 09:38 -0400, Rich Felker wrote:
Even though they have been moved under if() {} ? That's a tradeoff between patch readability and code readability :) I prefer the latter.
You can do the cosmetics in a separate subsequent patch. Mixing them makes it hard to review and hard for people going back later to audit.
That was not an isolated cosmetic change but an indentation change related to new functionality. I think it's better to keep those as part of the same patch as long as they don't reindent a block of several dozen lines.
It's been discussed before and the conclusion was along the lines of what I said. If you mix the cosmetics in the functional patch, someone MUST READ those lines and see that nothing actually changed. Surprisingly (or maybe not surprisingly) this is very difficult for the human eye.
Freetype can read fonts directly from memory, but fontconfig can't :(
Is there a reason we need fontconfig? I hate it...
Is there a reason you hate fontconfig? You hate various things, often for completely irrational reasons.
Yes, it's bloated and unnecessary. Rich
On Sunday 18 June 2006 17:38, Rich Felker wrote:
Why not just bypass fontconfig? I don't even have fontconfig and I'm quite happy with the way MPlayer works.
How exactly ? I've got font family, weight and slant values and need to get a font with most close characteristics. Ok, embedded subs often go with embedded fonts, I can open them all with FreeType and read their parameters. But what should I do if the script wants some standard font like Helvetica ? Reimplement fontconfig ?
Index: mencoder.c =================================================================== --- mencoder.c (.../branches/upstream) (revision 503) +++ mencoder.c (.../trunk) (revision 503) @@ -54,6 +54,7 @@ #include "libmpdemux/stheader.h" #include "libmpdemux/mp3_hdr.h" #include "libmpdemux/muxer.h" +#include "libmpdemux/matroska.h"
Why is this needed? Can it be avoided? (if this is in mplayer.c as well, same question there *g*)
For mkv_sh_sub_t definition.
Should be some way to fix this to be demuxer-independent.
It can be moved to demux_stream_t. Probably the same should be done with VobSub.
On Sun, Jun 18, 2006 at 09:05:19PM +0400, Evgeniy Stepanov wrote:
On Sunday 18 June 2006 17:38, Rich Felker wrote:
Why not just bypass fontconfig? I don't even have fontconfig and I'm quite happy with the way MPlayer works.
How exactly ? I've got font family, weight and slant values and need to get a font with most close characteristics. Ok, embedded subs often go with embedded fonts, I can open them all with FreeType and read their parameters. But what should I do if the script wants some standard font like Helvetica ?
Open all the fonts in the MPlayer fonts dir and read their characteristics? Anyway IMO it's nonsense for a file to request a font by name unless the font is embedded. How else can it expect it to be present?
Reimplement fontconfig ?
Since fontconfig is obviously deficient (cannot read from non-files) and also ugly bloatware, this is not such a bad idea as long as you do it right. However much less than fontconfig is needed...
For mkv_sh_sub_t definition.
Should be some way to fix this to be demuxer-independent.
It can be moved to demux_stream_t. Probably the same should be done with VobSub.
Sounds like a good idea. Rich
On Monday 19 June 2006 02:23, Rich Felker wrote:
Should be some way to fix this to be demuxer-independent.
It can be moved to demux_stream_t. Probably the same should be done with VobSub.
Sounds like a good idea.
And here is the implementation. I renamed mkv_sh_sub_t to sh_sub_t and moved it to demuxer.h. There is nothing matroska-specific in it.
On Sunday, 18 June 2006 at 15:38, Rich Felker wrote:
On Sun, Jun 18, 2006 at 02:33:13AM +0400, Evgeniy Stepanov wrote: [...]
+/** + * \brief Process embedded matroska font. Saves it to ~/.mplayer/fonts.
I do not like this at all. This feature might be misused in many ways, one I can think of is leaving a track record of which films somebody has been watching. The fact that it is disabled by default might make it acceptable though. I prefer if there was a way to use embedded fonts with at most using temporary files created via tmpfile instead (hopefully this function is portable enough).
Freetype can read fonts directly from memory, but fontconfig can't :(
Is there a reason we need fontconfig? I hate it...
But not everyone else on the planet does. ;)
I like to have a cache of fonts. What about creating persistent files with -embeddedfonts and temporary ones otherwise ? Do we need a mode when no files will be created at all ?
Yes absolutely. Creating files is a very very bad thing. It will not work on read-only filesystems (think live cd with MPlayer and movie files) and it leads to bugs and security issues more often than not.
Why not just bypass fontconfig? I don't even have fontconfig and I'm quite happy with the way MPlayer works.
I'm quite happy with fontconfig. I don't have to put path to the font file in MPlayer config. I don't even have to know which file corresponds to each font. So please don't insist on taking away that choice. Regards, R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Sun, Jun 18, 2006 at 07:29:06PM +0200, Dominik 'Rathann' Mierzejewski wrote:
I'm quite happy with fontconfig. I don't have to put path to the font file in MPlayer config. I don't even have to know which file corresponds to each font. So please don't insist on taking away that choice.
I didn't say support for fontconfig should be removed, just that it should not be required for SSA support. Rich
On Saturday 17 June 2006 23:27, Reimar Döffinger wrote:
And now for something completely different. What do you think would be needed to support this in vo_gl.c? What would be the steps needed to get one or several bitmaps containing the overlay data for the current frame?
- add VFCAP_EOSD - call ass_init after receiving VFCTRL_EOSD - call ass_render_frame for each frame The last two things should probably be done in vf_vo.c when the video-out has VFCAP_EOSD capability.
On Saturday 17 June 2006 23:27, Reimar Döffinger wrote:
Some reviewing and a question. Maybe even those developers without the time to review themselves can review my review...
I've fixed most problems discussed here since the review. The patch has been separated in 4 parts, they should be applied in the following order: 1. shsubt.patch 2. mplayer-ass.patch 3. cosmetics.patch The fourth, mencoder.patch contains changes to mencoder that are supposed to break something. shsubt.patch renames mkv_sh_sub_t to sh_sub_t and moves it to demuxer.h. This way I won't have to include matroska.h in mencoder.c. Looks like saving embedded fonts to temporary files is not a solution. tmpfile() does not give the name of the created file, mkstemp() does not delete it on exit. I'm thinking of opening fonts in memory with freetype and feeding their parameters to fontconfig. For now, I've kept the old behaviour. It's disabled by default.
Somehow I can't apply the latest version of the Patch. ***** mplayer-ass.patch ***** ============================= PATCH COMMAND: patch -p0 -g0 -E --no-backup-if-mismatch < /usr/local/overlay/portage/media-video/mplayer/files/mplayer-ass.patch ============================= patching file libmpcodecs/vf_ass.c patching file libmpcodecs/vfcap.h patching file libmpcodecs/Makefile Hunk #1 succeeded at 256 (offset -54 lines). patching file libmpcodecs/vf.c patching file libmpcodecs/vf.h patching file Makefile Hunk #2 succeeded at 113 with fuzz 2. Hunk #3 succeeded at 259 (offset 1 line). Hunk #4 succeeded at 290 (offset 1 line). Hunk #5 succeeded at 619 (offset 7 lines). patching file configure Hunk #1 succeeded at 248 (offset 3 lines). Hunk #2 succeeded at 1693 (offset 1 line). Hunk #4 succeeded at 5391 (offset -15 lines). Hunk #5 succeeded at 7547 (offset 29 lines). Hunk #6 succeeded at 8143 (offset 27 lines). patching file DOCS/man/en/mplayer.1 patching file mplayer.c Hunk #6 succeeded at 1992 with fuzz 2. Hunk #7 succeeded at 3451 (offset -96 lines). Hunk #8 succeeded at 3797 (offset -51 lines). Hunk #9 succeeded at 4366 (offset -28 lines). Hunk #10 succeeded at 5144 (offset 168 lines). patching file cfg-common.h Hunk #1 succeeded at 296 (offset 5 lines). patching file libass/ass_cache.h patching file libass/ass_types.h patching file libass/ass_fontconfig.c patching file libass/ass_mp.c patching file libass/ass_render.c patching file libass/ass.c patching file libass/ass_utils.c patching file libass/ass_fontconfig.h patching file libass/ass_mp.h patching file libass/ass_cache.c patching file libass/ass.h patching file libass/ass_utils.h patching file libass/Makefile patching file libmpdemux/demux_mkv.c Hunk #2 FAILED at 131. Hunk #4 FAILED at 194. Hunk #5 succeeded at 276 (offset -3 lines). Hunk #6 succeeded at 1437 (offset -3 lines). Hunk #7 succeeded at 2237 (offset -6 lines). Hunk #8 succeeded at 2399 (offset -6 lines). Hunk #9 succeeded at 2432 (offset -6 lines). Hunk #10 succeeded at 2527 (offset -17 lines). Hunk #11 succeeded at 2615 (offset -17 lines). Hunk #12 succeeded at 2635 (offset -17 lines). Hunk #13 succeeded at 2751 (offset -17 lines). 2 out of 13 hunks FAILED -- saving rejects to file libmpdemux/demux_mkv.c.rej patching file libmpdemux/demuxer.h Hunk #2 FAILED at 126. 1 out of 2 hunks FAILED -- saving rejects to file libmpdemux/demuxer.h.rej patching file libmpdemux/ebml.h patching file mencoder.c Hunk #1 succeeded at 219 (offset -4 lines). Hunk #2 succeeded at 649 (offset -4 lines). Hunk #3 succeeded at 687 (offset -4 lines). Hunk #4 succeeded at 878 (offset -4 lines). Hunk #5 succeeded at 1372 (offset -4 lines). Hunk #6 succeeded at 1788 (offset -4 lines). ============================= PATCH COMMAND: patch -p1 -g0 -E --no-backup-if-mismatch < /usr/local/overlay/portage/media-video/mplayer/files/mplayer-ass.patch ============================= patching file vf_ass.c can't find file to patch at input line 444 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vfcap.h |=================================================================== |--- libmpcodecs/vfcap.h (.../tmp2) (revision 542) |+++ libmpcodecs/vfcap.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored patching file Makefile Hunk #1 FAILED at 310. 1 out of 1 hunk FAILED -- saving rejects to file Makefile.rej can't find file to patch at input line 470 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.c |=================================================================== |--- libmpcodecs/vf.c (.../tmp2) (revision 542) |+++ libmpcodecs/vf.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored can't find file to patch at input line 492 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.h |=================================================================== |--- libmpcodecs/vf.h (.../tmp2) (revision 542) |+++ libmpcodecs/vf.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 504 of patch can't find file to patch at input line 504 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: Makefile |=================================================================== |--- Makefile (.../tmp2) (revision 542) |+++ Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 5 out of 5 hunks ignored missing header for unified diff at line 559 of patch can't find file to patch at input line 559 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: configure |=================================================================== |--- configure (.../tmp2) (revision 542) |+++ configure (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored can't find file to patch at input line 642 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: DOCS/man/en/mplayer.1 |=================================================================== |--- DOCS/man/en/mplayer.1 (.../tmp2) (revision 542) |+++ DOCS/man/en/mplayer.1 (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 689 of patch can't find file to patch at input line 689 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mplayer.c |=================================================================== |--- mplayer.c (.../tmp2) (revision 542) |+++ mplayer.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 10 out of 10 hunks ignored missing header for unified diff at line 894 of patch can't find file to patch at input line 894 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: cfg-common.h |=================================================================== |--- cfg-common.h (.../tmp2) (revision 542) |+++ cfg-common.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored patching file ass_cache.h patching file ass_types.h patching file ass_fontconfig.c patching file ass_mp.c patching file ass_render.c patching file ass.c patching file ass_utils.c patching file ass_fontconfig.h patching file ass_mp.h patching file ass_cache.c patching file ass.h patching file ass_utils.h patching file Makefile can't find file to patch at input line 4579 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demux_mkv.c |=================================================================== |--- libmpdemux/demux_mkv.c (.../tmp2) (revision 542) |+++ libmpdemux/demux_mkv.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 13 out of 13 hunks ignored can't find file to patch at input line 4882 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demuxer.h |=================================================================== |--- libmpdemux/demuxer.h (.../tmp2) (revision 542) |+++ libmpdemux/demuxer.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored can't find file to patch at input line 4915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/ebml.h |=================================================================== |--- libmpdemux/ebml.h (.../tmp2) (revision 542) |+++ libmpdemux/ebml.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4932 of patch can't find file to patch at input line 4932 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mencoder.c |=================================================================== |--- mencoder.c (.../tmp2) (revision 542) |+++ mencoder.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored ============================= PATCH COMMAND: patch -p2 -g0 -E --no-backup-if-mismatch < /usr/local/overlay/portage/media-video/mplayer/files/mplayer-ass.patch ============================= missing header for unified diff at line 5 of patch can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf_ass.c |=================================================================== |--- libmpcodecs/vf_ass.c (.../tmp2) (revision 0) |+++ libmpcodecs/vf_ass.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 444 of patch can't find file to patch at input line 444 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vfcap.h |=================================================================== |--- libmpcodecs/vfcap.h (.../tmp2) (revision 542) |+++ libmpcodecs/vfcap.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 455 of patch can't find file to patch at input line 455 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/Makefile |=================================================================== |--- libmpcodecs/Makefile (.../tmp2) (revision 542) |+++ libmpcodecs/Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 470 of patch can't find file to patch at input line 470 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.c |=================================================================== |--- libmpcodecs/vf.c (.../tmp2) (revision 542) |+++ libmpcodecs/vf.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 492 of patch can't find file to patch at input line 492 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.h |=================================================================== |--- libmpcodecs/vf.h (.../tmp2) (revision 542) |+++ libmpcodecs/vf.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 504 of patch can't find file to patch at input line 504 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: Makefile |=================================================================== |--- Makefile (.../tmp2) (revision 542) |+++ Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 5 out of 5 hunks ignored missing header for unified diff at line 559 of patch can't find file to patch at input line 559 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: configure |=================================================================== |--- configure (.../tmp2) (revision 542) |+++ configure (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored can't find file to patch at input line 642 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: DOCS/man/en/mplayer.1 |=================================================================== |--- DOCS/man/en/mplayer.1 (.../tmp2) (revision 542) |+++ DOCS/man/en/mplayer.1 (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 689 of patch can't find file to patch at input line 689 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mplayer.c |=================================================================== |--- mplayer.c (.../tmp2) (revision 542) |+++ mplayer.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 10 out of 10 hunks ignored missing header for unified diff at line 894 of patch can't find file to patch at input line 894 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: cfg-common.h |=================================================================== |--- cfg-common.h (.../tmp2) (revision 542) |+++ cfg-common.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 915 of patch can't find file to patch at input line 915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_cache.h |=================================================================== |--- libass/ass_cache.h (.../tmp2) (revision 0) |+++ libass/ass_cache.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 970 of patch can't find file to patch at input line 970 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_types.h |=================================================================== |--- libass/ass_types.h (.../tmp2) (revision 0) |+++ libass/ass_types.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1058 of patch can't find file to patch at input line 1058 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_fontconfig.c |=================================================================== |--- libass/ass_fontconfig.c (.../tmp2) (revision 0) |+++ libass/ass_fontconfig.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1255 of patch can't find file to patch at input line 1255 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_mp.c |=================================================================== |--- libass/ass_mp.c (.../tmp2) (revision 0) |+++ libass/ass_mp.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1269 of patch can't find file to patch at input line 1269 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_render.c |=================================================================== |--- libass/ass_render.c (.../tmp2) (revision 0) |+++ libass/ass_render.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 3039 of patch can't find file to patch at input line 3039 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass.c |=================================================================== |--- libass/ass.c (.../tmp2) (revision 0) |+++ libass/ass.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4017 of patch can't find file to patch at input line 4017 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_utils.c |=================================================================== |--- libass/ass_utils.c (.../tmp2) (revision 0) |+++ libass/ass_utils.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4115 of patch can't find file to patch at input line 4115 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_fontconfig.h |=================================================================== |--- libass/ass_fontconfig.h (.../tmp2) (revision 0) |+++ libass/ass_fontconfig.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4131 of patch can't find file to patch at input line 4131 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_mp.h |=================================================================== |--- libass/ass_mp.h (.../tmp2) (revision 0) |+++ libass/ass_mp.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4147 of patch can't find file to patch at input line 4147 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_cache.c |=================================================================== |--- libass/ass_cache.c (.../tmp2) (revision 0) |+++ libass/ass_cache.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4361 of patch can't find file to patch at input line 4361 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass.h |=================================================================== |--- libass/ass.h (.../tmp2) (revision 0) |+++ libass/ass.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4508 of patch can't find file to patch at input line 4508 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_utils.h |=================================================================== |--- libass/ass_utils.h (.../tmp2) (revision 0) |+++ libass/ass_utils.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4524 of patch can't find file to patch at input line 4524 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/Makefile |=================================================================== |--- libass/Makefile (.../tmp2) (revision 0) |+++ libass/Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4579 of patch can't find file to patch at input line 4579 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demux_mkv.c |=================================================================== |--- libmpdemux/demux_mkv.c (.../tmp2) (revision 542) |+++ libmpdemux/demux_mkv.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 13 out of 13 hunks ignored missing header for unified diff at line 4882 of patch can't find file to patch at input line 4882 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demuxer.h |=================================================================== |--- libmpdemux/demuxer.h (.../tmp2) (revision 542) |+++ libmpdemux/demuxer.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 4915 of patch can't find file to patch at input line 4915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/ebml.h |=================================================================== |--- libmpdemux/ebml.h (.../tmp2) (revision 542) |+++ libmpdemux/ebml.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4932 of patch can't find file to patch at input line 4932 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mencoder.c |=================================================================== |--- mencoder.c (.../tmp2) (revision 542) |+++ mencoder.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored ============================= PATCH COMMAND: patch -p3 -g0 -E --no-backup-if-mismatch < /usr/local/overlay/portage/media-video/mplayer/files/mplayer-ass.patch ============================= missing header for unified diff at line 5 of patch can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf_ass.c |=================================================================== |--- libmpcodecs/vf_ass.c (.../tmp2) (revision 0) |+++ libmpcodecs/vf_ass.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 444 of patch can't find file to patch at input line 444 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vfcap.h |=================================================================== |--- libmpcodecs/vfcap.h (.../tmp2) (revision 542) |+++ libmpcodecs/vfcap.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 455 of patch can't find file to patch at input line 455 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/Makefile |=================================================================== |--- libmpcodecs/Makefile (.../tmp2) (revision 542) |+++ libmpcodecs/Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 470 of patch can't find file to patch at input line 470 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.c |=================================================================== |--- libmpcodecs/vf.c (.../tmp2) (revision 542) |+++ libmpcodecs/vf.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 492 of patch can't find file to patch at input line 492 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.h |=================================================================== |--- libmpcodecs/vf.h (.../tmp2) (revision 542) |+++ libmpcodecs/vf.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 504 of patch can't find file to patch at input line 504 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: Makefile |=================================================================== |--- Makefile (.../tmp2) (revision 542) |+++ Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 5 out of 5 hunks ignored missing header for unified diff at line 559 of patch can't find file to patch at input line 559 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: configure |=================================================================== |--- configure (.../tmp2) (revision 542) |+++ configure (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored can't find file to patch at input line 642 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: DOCS/man/en/mplayer.1 |=================================================================== |--- DOCS/man/en/mplayer.1 (.../tmp2) (revision 542) |+++ DOCS/man/en/mplayer.1 (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 689 of patch can't find file to patch at input line 689 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mplayer.c |=================================================================== |--- mplayer.c (.../tmp2) (revision 542) |+++ mplayer.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 10 out of 10 hunks ignored missing header for unified diff at line 894 of patch can't find file to patch at input line 894 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: cfg-common.h |=================================================================== |--- cfg-common.h (.../tmp2) (revision 542) |+++ cfg-common.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 915 of patch can't find file to patch at input line 915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_cache.h |=================================================================== |--- libass/ass_cache.h (.../tmp2) (revision 0) |+++ libass/ass_cache.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 970 of patch can't find file to patch at input line 970 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_types.h |=================================================================== |--- libass/ass_types.h (.../tmp2) (revision 0) |+++ libass/ass_types.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1058 of patch can't find file to patch at input line 1058 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_fontconfig.c |=================================================================== |--- libass/ass_fontconfig.c (.../tmp2) (revision 0) |+++ libass/ass_fontconfig.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1255 of patch can't find file to patch at input line 1255 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_mp.c |=================================================================== |--- libass/ass_mp.c (.../tmp2) (revision 0) |+++ libass/ass_mp.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1269 of patch can't find file to patch at input line 1269 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_render.c |=================================================================== |--- libass/ass_render.c (.../tmp2) (revision 0) |+++ libass/ass_render.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 3039 of patch can't find file to patch at input line 3039 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass.c |=================================================================== |--- libass/ass.c (.../tmp2) (revision 0) |+++ libass/ass.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4017 of patch can't find file to patch at input line 4017 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_utils.c |=================================================================== |--- libass/ass_utils.c (.../tmp2) (revision 0) |+++ libass/ass_utils.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4115 of patch can't find file to patch at input line 4115 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_fontconfig.h |=================================================================== |--- libass/ass_fontconfig.h (.../tmp2) (revision 0) |+++ libass/ass_fontconfig.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4131 of patch can't find file to patch at input line 4131 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_mp.h |=================================================================== |--- libass/ass_mp.h (.../tmp2) (revision 0) |+++ libass/ass_mp.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4147 of patch can't find file to patch at input line 4147 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_cache.c |=================================================================== |--- libass/ass_cache.c (.../tmp2) (revision 0) |+++ libass/ass_cache.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4361 of patch can't find file to patch at input line 4361 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass.h |=================================================================== |--- libass/ass.h (.../tmp2) (revision 0) |+++ libass/ass.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4508 of patch can't find file to patch at input line 4508 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_utils.h |=================================================================== |--- libass/ass_utils.h (.../tmp2) (revision 0) |+++ libass/ass_utils.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4524 of patch can't find file to patch at input line 4524 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/Makefile |=================================================================== |--- libass/Makefile (.../tmp2) (revision 0) |+++ libass/Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4579 of patch can't find file to patch at input line 4579 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demux_mkv.c |=================================================================== |--- libmpdemux/demux_mkv.c (.../tmp2) (revision 542) |+++ libmpdemux/demux_mkv.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 13 out of 13 hunks ignored missing header for unified diff at line 4882 of patch can't find file to patch at input line 4882 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demuxer.h |=================================================================== |--- libmpdemux/demuxer.h (.../tmp2) (revision 542) |+++ libmpdemux/demuxer.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 4915 of patch can't find file to patch at input line 4915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/ebml.h |=================================================================== |--- libmpdemux/ebml.h (.../tmp2) (revision 542) |+++ libmpdemux/ebml.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4932 of patch can't find file to patch at input line 4932 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mencoder.c |=================================================================== |--- mencoder.c (.../tmp2) (revision 542) |+++ mencoder.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored ============================= PATCH COMMAND: patch -p4 -g0 -E --no-backup-if-mismatch < /usr/local/overlay/portage/media-video/mplayer/files/mplayer-ass.patch ============================= missing header for unified diff at line 5 of patch can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf_ass.c |=================================================================== |--- libmpcodecs/vf_ass.c (.../tmp2) (revision 0) |+++ libmpcodecs/vf_ass.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 444 of patch can't find file to patch at input line 444 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vfcap.h |=================================================================== |--- libmpcodecs/vfcap.h (.../tmp2) (revision 542) |+++ libmpcodecs/vfcap.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 455 of patch can't find file to patch at input line 455 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/Makefile |=================================================================== |--- libmpcodecs/Makefile (.../tmp2) (revision 542) |+++ libmpcodecs/Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 470 of patch can't find file to patch at input line 470 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.c |=================================================================== |--- libmpcodecs/vf.c (.../tmp2) (revision 542) |+++ libmpcodecs/vf.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 492 of patch can't find file to patch at input line 492 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpcodecs/vf.h |=================================================================== |--- libmpcodecs/vf.h (.../tmp2) (revision 542) |+++ libmpcodecs/vf.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 504 of patch can't find file to patch at input line 504 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: Makefile |=================================================================== |--- Makefile (.../tmp2) (revision 542) |+++ Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 5 out of 5 hunks ignored missing header for unified diff at line 559 of patch can't find file to patch at input line 559 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: configure |=================================================================== |--- configure (.../tmp2) (revision 542) |+++ configure (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored missing header for unified diff at line 642 of patch can't find file to patch at input line 642 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: DOCS/man/en/mplayer.1 |=================================================================== |--- DOCS/man/en/mplayer.1 (.../tmp2) (revision 542) |+++ DOCS/man/en/mplayer.1 (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 689 of patch can't find file to patch at input line 689 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mplayer.c |=================================================================== |--- mplayer.c (.../tmp2) (revision 542) |+++ mplayer.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 10 out of 10 hunks ignored missing header for unified diff at line 894 of patch can't find file to patch at input line 894 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: cfg-common.h |=================================================================== |--- cfg-common.h (.../tmp2) (revision 542) |+++ cfg-common.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 915 of patch can't find file to patch at input line 915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_cache.h |=================================================================== |--- libass/ass_cache.h (.../tmp2) (revision 0) |+++ libass/ass_cache.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 970 of patch can't find file to patch at input line 970 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_types.h |=================================================================== |--- libass/ass_types.h (.../tmp2) (revision 0) |+++ libass/ass_types.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1058 of patch can't find file to patch at input line 1058 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_fontconfig.c |=================================================================== |--- libass/ass_fontconfig.c (.../tmp2) (revision 0) |+++ libass/ass_fontconfig.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1255 of patch can't find file to patch at input line 1255 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_mp.c |=================================================================== |--- libass/ass_mp.c (.../tmp2) (revision 0) |+++ libass/ass_mp.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 1269 of patch can't find file to patch at input line 1269 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_render.c |=================================================================== |--- libass/ass_render.c (.../tmp2) (revision 0) |+++ libass/ass_render.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 3039 of patch can't find file to patch at input line 3039 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass.c |=================================================================== |--- libass/ass.c (.../tmp2) (revision 0) |+++ libass/ass.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4017 of patch can't find file to patch at input line 4017 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_utils.c |=================================================================== |--- libass/ass_utils.c (.../tmp2) (revision 0) |+++ libass/ass_utils.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4115 of patch can't find file to patch at input line 4115 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_fontconfig.h |=================================================================== |--- libass/ass_fontconfig.h (.../tmp2) (revision 0) |+++ libass/ass_fontconfig.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4131 of patch can't find file to patch at input line 4131 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_mp.h |=================================================================== |--- libass/ass_mp.h (.../tmp2) (revision 0) |+++ libass/ass_mp.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4147 of patch can't find file to patch at input line 4147 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_cache.c |=================================================================== |--- libass/ass_cache.c (.../tmp2) (revision 0) |+++ libass/ass_cache.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4361 of patch can't find file to patch at input line 4361 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass.h |=================================================================== |--- libass/ass.h (.../tmp2) (revision 0) |+++ libass/ass.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4508 of patch can't find file to patch at input line 4508 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/ass_utils.h |=================================================================== |--- libass/ass_utils.h (.../tmp2) (revision 0) |+++ libass/ass_utils.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4524 of patch can't find file to patch at input line 4524 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libass/Makefile |=================================================================== |--- libass/Makefile (.../tmp2) (revision 0) |+++ libass/Makefile (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4579 of patch can't find file to patch at input line 4579 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demux_mkv.c |=================================================================== |--- libmpdemux/demux_mkv.c (.../tmp2) (revision 542) |+++ libmpdemux/demux_mkv.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 13 out of 13 hunks ignored missing header for unified diff at line 4882 of patch can't find file to patch at input line 4882 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/demuxer.h |=================================================================== |--- libmpdemux/demuxer.h (.../tmp2) (revision 542) |+++ libmpdemux/demuxer.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 2 out of 2 hunks ignored missing header for unified diff at line 4915 of patch can't find file to patch at input line 4915 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: libmpdemux/ebml.h |=================================================================== |--- libmpdemux/ebml.h (.../tmp2) (revision 542) |+++ libmpdemux/ebml.h (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored missing header for unified diff at line 4932 of patch can't find file to patch at input line 4932 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: mencoder.c |=================================================================== |--- mencoder.c (.../tmp2) (revision 542) |+++ mencoder.c (.../stable) (revision 542) -------------------------- No file to patch. Skipping patch. 6 out of 6 hunks ignored
On Sunday 25 June 2006 16:45, seeboo wrote:
Somehow I can't apply the latest version of the Patch.
I've just rechecked - it applies cleanly to the current svn. Maybe you are doing it in the wrong order ? Once again: 1. shsubt.patch 2. mplayer-ass.patch 3. cosmetics.patch An unrelated issue: mencoder-ass.patch was included in mplayer-ass.patch by mistake. Fixed versions attached.
On Sun, 2006-06-25 at 17:09 +0400, Evgeniy Stepanov wrote:
3. cosmetics.patch
IMO handling a separate patch for this is much more trouble than including it in the main patch (yes I know some people asked to have it separately...).
An unrelated issue: mencoder-ass.patch was included in mplayer-ass.patch by mistake. Fixed versions attached.
I haven't looked at the latest versions yet, but remembered an issue that I forgot to mention in my previous comments: + images = ass_render_frame(vf->priv->ass_priv, ass_track, (pts+sub_delay) * 1000); It would be better to round the timestamp value instead of the current truncating cast to long long. If the original video was mkv with 1 ms timestamp precision and the subtitles were timed to the exact frame timestamps it could matter. Converting the original timestamp to double and then back with the truncating conversion here can produce a smaller-than-original value. It'll happen for almost all timestamps if the fpu is in round-down mode and for some in round-to-nearest mode (most likely case). Adding rounding here is guaranteed to reproduce the original value as long as it isn't so insanely huge it would exceed the precision of doubles.
On Sunday 25 June 2006 18:02, Uoti Urpala wrote:
IMO handling a separate patch for this is much more trouble than including it in the main patch (yes I know some people asked to have it separately...).
It seems that the multipatch version will satisfy more people. However, it is just my perception. We should probably put it to the vote.
+ images = ass_render_frame(vf->priv->ass_priv, ass_track, (pts+sub_delay) * 1000);
It would be better to round the timestamp value instead of the current truncating cast to long long. [...]
Fixed. The patches are starting to multiply, there is now 6 of them. 1. shsubt.patch Moves mkv_sh_sub_t to demuxer.h and renames it to sh_sub_t. 2. utf8_get_char.patch Makes utf8_get_char() non-static. It is used in libass. 3. guess_cp.patch Splits guess_cp() in two functions: - guess_buffer_cp() determines the encoding of text, given a pointer to a buffer and its length; - guess_cp() reads some data from stream and calls guess_buffer_cp() guess_buffer_cp() is used in libass. 4. mplayer-ass.patch Everything else is here. 5. cosmetics.patch 6. mencoder-ass.patch Man page is updated, some minor bugs fixed.
On Sunday, 02 July 2006 at 22:42, Evgeniy Stepanov wrote:
The patches are starting to multiply, there is now 6 of them.
Don't worry, that's what makes them easier to apply separately.
1. shsubt.patch Moves mkv_sh_sub_t to demuxer.h and renames it to sh_sub_t. 2. utf8_get_char.patch Makes utf8_get_char() non-static. It is used in libass. 3. guess_cp.patch Splits guess_cp() in two functions: - guess_buffer_cp() determines the encoding of text, given a pointer to a buffer and its length; - guess_cp() reads some data from stream and calls guess_buffer_cp() guess_buffer_cp() is used in libass. 4. mplayer-ass.patch Everything else is here. 5. cosmetics.patch
I've been using this patch for a while now with little or no problems at all, so I'm going to apply 1-5 soon if nobody objects. Hopefully after some comments from Moritz on demux_mkv changes.
6. mencoder-ass.patch
Apparently this doesn't work well. It's not this patch's fault but rather general mencoder pts mishandling. I'll probably apply this after that is fixed.
Man page is updated, some minor bugs fixed.
Great. Regards, R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Monday 03 July 2006 18:56, Dominik 'Rathann' Mierzejewski wrote:
I've been using this patch for a while now with little or no problems at all, so I'm going to apply 1-5 soon if nobody objects. Hopefully after some comments from Moritz on demux_mkv changes.
Can I get maintainer status and svn write access ? I'd like to apply it myself if possible.
Hi, On Wednesday 05 July 2006 00:01, Evgeniy Stepanov wrote:
Can I get maintainer status and svn write access ? I'd like to apply it myself if possible.
Have you considered extend the ssa support to a general subtitle support, recently I did a little hack to make mplayer support embeded mp4 subtitles, at the moment it is based on the old subtitle engine, so all the font/color/position settings are ignore. Suppose I want to take advantage of the ssa engine, is there a easy way to do this with the current implementation? You can found a sample of the mp4 file here: http://rapidshare.de/files/5737885/1Video_2Audio_2SUBs_timed_text_streams_.m... and document from apple: http://developer.apple.com/documentation/QuickTime/QT6_3/index.html the 'tx3g' part. also a introduction in wikipedia: http://en.wikipedia.org/wiki/MPEG-4_Part_17 -- Best Regards, LR
On Wednesday 05 July 2006 06:43, 陆然 wrote:
Have you considered extend the ssa support to a general subtitle support, recently I did a little hack to make mplayer support embeded mp4 subtitles, at the moment it is based on the old subtitle engine, so all the font/color/position settings are ignore. Suppose I want to take advantage of the ssa engine, is there a easy way to do this with the current implementation?
You can found a sample of the mp4 file here: http://rapidshare.de/files/5737885/1Video_2Audio_2SUBs_timed_text_streams_. mp4.html and document from apple: http://developer.apple.com/documentation/QuickTime/QT6_3/index.html the 'tx3g' part. also a introduction in wikipedia: http://en.wikipedia.org/wiki/MPEG-4_Part_17
Sure. I've attached a small function for converting subdata to ass_track. This way, every subtitle format supported by subreader can be displayed with libass. Something like this can be done for mp4 timed text, too.
Hi, On Wednesday 05 July 2006 15:54, Evgeniy Stepanov wrote:
Sure. I've attached a small function for converting subdata to ass_track. This way, every subtitle format supported by subreader can be displayed with libass. Something like this can be done for mp4 timed text, too.
All right. So all I need is to extract all the subtitles in a file and feed them to a ass_read_subdata? That's great. BTW, I see the old sub system read a subtitle embeded in the ogg/mkv files only when it is needed to be displayed, and the new ssa code in mkv reads the subtitles once and for all. Is there any reason for this difference? -- Best Regards, LR
?? wrote:
Hi,
On Wednesday 05 July 2006 15:54, Evgeniy Stepanov wrote:
Sure. I've attached a small function for converting subdata to ass_track. This way, every subtitle format supported by subreader can be displayed with libass. Something like this can be done for mp4 timed text, too.
All right. So all I need is to extract all the subtitles in a file and feed them to a ass_read_subdata? That's great.
BTW, I see the old sub system read a subtitle embeded in the ogg/mkv files only when it is needed to be displayed, and the new ssa code in mkv reads the subtitles once and for all. Is there any reason for this difference?
it's only one more indication that the current subtitles code in mplayer should be destroyed ASAP -- Email.it, the professional e-mail, gratis per te: http://www.email.it/f Sponsor: Ascolta tutta la musica che vuoi gratis! * Clicca su www.radiosnj.com Clicca qui: http://adv.email.it/cgi-bin/foclick.cgi?mid=5176&d=5-7
On Wed, Jul 05, 2006 at 06:48:07PM +0800, ?????? wrote:
Hi,
On Wednesday 05 July 2006 15:54, Evgeniy Stepanov wrote:
Sure. I've attached a small function for converting subdata to ass_track. This way, every subtitle format supported by subreader can be displayed with libass. Something like this can be done for mp4 timed text, too.
All right. So all I need is to extract all the subtitles in a file and feed them to a ass_read_subdata? That's great.
Arrg, why "in a file"?? Can't you just pass the fonts directly without storing them to disk? If there's any way around it I'm strongly against features that require temp files. Rich
Hi Rich, On 05/07/06, Rich Felker <dalias@aerifal.cx> wrote:
On Wed, Jul 05, 2006 at 06:48:07PM +0800, 陆然 wrote:
Hi,
On Wednesday 05 July 2006 15:54, Evgeniy Stepanov wrote:
Sure. I've attached a small function for converting subdata to ass_track. This way, every subtitle format supported by subreader can be displayed with libass. Something like this can be done for mp4 timed text, too.
All right. So all I need is to extract all the subtitles in a file and feed them to a ass_read_subdata? That's great.
Arrg, why "in a file"?? Can't you just pass the fonts directly without storing them to disk?
I don't think that is what 陆然 said. The conversation was about subtitles embedded in a container format like MP4. The subtitles are in a file and need to be separated (extracted) from the audio/video data, before passing it to ass_read_subdata. Besides, no patch was sent that created those temp files, so should you care what others do with MPlayers GPL'd code in the privacy of their own homes? -V
Hi, On Wednesday 05 July 2006 23:33, Rich Felker wrote:
All right. So all I need is to extract all the subtitles in a file and feed them to a ass_read_subdata? That's great. Arrg, why "in a file"?? Can't you just pass the fonts directly without storing them to disk?
Well, I mean extract the subtitles embeded in the movie, not to a temp file :) -- Best Regards, LR
On Wednesday 05 July 2006 14:48, 陆然 wrote:
Hi,
On Wednesday 05 July 2006 15:54, Evgeniy Stepanov wrote:
Sure. I've attached a small function for converting subdata to ass_track. This way, every subtitle format supported by subreader can be displayed with libass. Something like this can be done for mp4 timed text, too.
All right. So all I need is to extract all the subtitles in a file and feed them to a ass_read_subdata? That's great.
You won't be able to get multicolored subtitles this way. It is better to write your own function using ass_read_subdata as an example.
BTW, I see the old sub system read a subtitle embeded in the ogg/mkv files only when it is needed to be displayed, and the new ssa code in mkv reads the subtitles once and for all. Is there any reason for this difference?
Not exactly. New system adds events to ass_track as they are found in the stream and keeps them forever. The old system only keeps the latest event, which is clearly not enough with ssa. You can't extract _all_ subtitles in a file until it is completely demuxed. So you should allocate an ass_track for each mp4 subtitle track, and update it with subs coming from the demuxer. In matroska/ssa case it is implemented in ass_process_chunk() and ass_process_line().
Hi, On Wednesday 05 July 2006 23:53, Evgeniy Stepanov wrote:
You won't be able to get multicolored subtitles this way. It is better to write your own function using ass_read_subdata as an example.
No problem, It's could be a start point ;)
Not exactly. New system adds events to ass_track as they are found in the stream and keeps them forever. The old system only keeps the latest event, which is clearly not enough with ssa.
You can't extract _all_ subtitles in a file until it is completely demuxed. So you should allocate an ass_track for each mp4 subtitle track, and update it with subs coming from the demuxer. In matroska/ssa case it is implemented in ass_process_chunk() and ass_process_line().
So it's the limit of the container? Then MP4 is special I think, In MP4 the every track is completely indexed, in fact MPlayer already generated all the infomation needed to generate every subtitle (start time, offset in the file...). Audio & video track is well indexed too, thus when demux_fill_buffer is called, it simply fill the requied audio/video track, subtitle track will not be demuxed "by the way". That's why I prefer get subtitles in the open process. If there is too many subtitles the delay might be noticable in this approach though. -- Best Regards, LR
On Thu, Jul 06, 2006 at 09:58:43AM +0800, ?????? wrote:
Hi,
On Wednesday 05 July 2006 23:53, Evgeniy Stepanov wrote:
You won't be able to get multicolored subtitles this way. It is better to write your own function using ass_read_subdata as an example.
No problem, It's could be a start point ;)
Not exactly. New system adds events to ass_track as they are found in the stream and keeps them forever. The old system only keeps the latest event, which is clearly not enough with ssa.
You can't extract _all_ subtitles in a file until it is completely demuxed. So you should allocate an ass_track for each mp4 subtitle track, and update it with subs coming from the demuxer. In matroska/ssa case it is implemented in ass_process_chunk() and ass_process_line().
So it's the limit of the container? Then MP4 is special I think, In MP4 the every track is completely indexed, in fact MPlayer already generated all the
'special' i guess is one way to put it. what do you do if the index is damaged or missing? for a file format to be any good you need to be able to read it without the index. imo the super-extensive index of mp4 is just bloat, making the overhead for the file very large.
infomation needed to generate every subtitle (start time, offset in the file...). Audio & video track is well indexed too, thus when demux_fill_buffer is called, it simply fill the requied audio/video track, subtitle track will not be demuxed "by the way". That's why I prefer get subtitles in the open process. If there is too many subtitles the delay might be noticable in this approach though.
yes, in general i would think the performance impact is unacceptable. especially consider reading off slow media like cdrom/dvd, where seeking is slow. rich
Hi, On 7/5/06, Evgeniy Stepanov <eugeni.stepanov@gmail.com> wrote:
Not exactly. New system adds events to ass_track as they are found in the stream and keeps them forever. The old system only keeps the latest event, which is clearly not enough with ssa.
You can't extract _all_ subtitles in a file until it is completely demuxed. So you should allocate an ass_track for each mp4 subtitle track, and update it with subs coming from the demuxer. In matroska/ssa case it is implemented in ass_process_chunk() and ass_process_line(). _______________________________________________
I've tried to implement a function similar to ass_process_line() to generate the ass events, but I get a weird "FT_Glyph_To_Bitmap error: -1 -1,symbol 74,index 40", I've not modify the ass code, so I guess I missed some style setup. Any hints? -- Best Regard, LR
On Thursday 06 July 2006 20:47, Lu Ran wrote:
I've tried to implement a function similar to ass_process_line() to generate the ass events, but I get a weird "FT_Glyph_To_Bitmap error: -1 -1,symbol 74,index 40", I've not modify the ass code, so I guess I missed some style setup. Any hints?
It must be something related to glyph bitmap size, check ScaleX/ScaleY and FontSize.
Hi, On Friday 07 July 2006 01:03, Evgeniy Stepanov wrote:
It must be something related to glyph bitmap size, check ScaleX/ScaleY and FontSize.
Indeed, I need to setup ScaleX/ScaleY a default value, do I need to set all the items in a style structure? some does not have a counter part in the mov style, like ScaleX/ScaleY. -- Best Regards, LR
On Friday 07 July 2006 05:52, 陆然 wrote:
Hi,
On Friday 07 July 2006 01:03, Evgeniy Stepanov wrote:
It must be something related to glyph bitmap size, check ScaleX/ScaleY and FontSize.
Indeed, I need to setup ScaleX/ScaleY a default value, do I need to set all the items in a style structure? some does not have a counter part in the mov style, like ScaleX/ScaleY.
Setting these fields should be sufficient: Name, FontName, FontSize, ScaleX/Y, BorderStyle, Alignment, Outline, Colors and Margins.
On Fri, Jul 07, 2006 at 01:22:01PM +0400, Evgeniy Stepanov wrote:
On Friday 07 July 2006 05:52, ?????? wrote:
On Friday 07 July 2006 01:03, Evgeniy Stepanov wrote:
It must be something related to glyph bitmap size, check ScaleX/ScaleY and FontSize.
Indeed, I need to setup ScaleX/ScaleY a default value, do I need to set all the items in a style structure? some does not have a counter part in the mov style, like ScaleX/ScaleY.
Setting these fields should be sufficient: Name, FontName, FontSize, ScaleX/Y, BorderStyle, Alignment, Outline, Colors and Margins.
BTW, Evgeniy has a Subversion account now. Is there anything stopping him from committing this? Diego
On Friday 07 July 2006 13:24, Diego Biurrun wrote:
BTW, Evgeniy has a Subversion account now. Is there anything stopping him from committing this?
Ok, if nobody objects, I'm commiting everything except mencoder-ass.patch in a few hours.
On Fri, 2006-07-07 at 18:36 +0400, Evgeniy Stepanov wrote:
Ok, if nobody objects, I'm commiting everything except mencoder-ass.patch in a few hours.
IIRC linking failed when I tried to apply the other patches without that one earlier because some variables were defined identically in mplayer.c and mencoder.c and were referred to in other object files that were linked in both binaries. There are a few alternatives: 1) Apply the mencoder patch too as is. Might or might not break things. Feeding the timestamps to the filter chain is the problematic part; feeding them broke encoding with lavc earlier, but those weren't exactly the same ones then (they were from the muxer, the patch uses sh_video->pts). 2) Apply the mencoder patch otherwise but leave out the timestamps, doesn't break things but leaves the added code useless for now as the filter won't work under mencoder. 3) Add only the variable definitions to mencoder.c but leave out the rest of the patch. 4) Move the needed variable definitions to some other file that gets linked to both the mplayer and mencoder binaries. I'm currently not sure which choice would be best and probably won't try to investigate it further during the next few hours.
On Friday 07 July 2006 18:58, Uoti Urpala wrote:
IIRC linking failed when I tried to apply the other patches without that one earlier because some variables were defined identically in mplayer.c and mencoder.c and were referred to in other object files that were linked in both binaries. There are a few alternatives: 1) Apply the mencoder patch too as is. Might or might not break things. Feeding the timestamps to the filter chain is the problematic part; feeding them broke encoding with lavc earlier, but those weren't exactly the same ones then (they were from the muxer, the patch uses sh_video->pts).
I've tried encoding short clips recently and didn't experience any problems. What is the exact problem and what can be done to fix it ? Is it somehow related to your "--correct-pts" patch ?
2) Apply the mencoder patch otherwise but leave out the timestamps, doesn't break things but leaves the added code useless for now as the filter won't work under mencoder. 3) Add only the variable definitions to mencoder.c but leave out the rest of the patch. 4) Move the needed variable definitions to some other file that gets linked to both the mplayer and mencoder binaries.
I'm currently not sure which choice would be best and probably won't try to investigate it further during the next few hours.
I prefer the 3rd option, along with moving ass_enabled to ass_mp.c.
Committed.
On Fri, 2006-07-07 at 21:50 +0400, Evgeniy Stepanov wrote:
On Friday 07 July 2006 18:58, Uoti Urpala wrote:
Feeding the timestamps to the filter chain is the problematic part; feeding them broke encoding with lavc earlier, but those weren't exactly the same ones then (they were from the muxer, the patch uses sh_video->pts).
I've tried encoding short clips recently and didn't experience any problems. What is the exact problem and what can be done to fix it ? Is it somehow related to your "--correct-pts" patch ?
The cause of the previous problem was that mencoder was feeding bad timestamps into the filter chain and the lavc encoder choked on those. Without any timestamps it assumes a constant framerate and mostly works. To fully fix the timestamp handling would require several changes in mencoder, and also (for "-ovc copy" at least) in the built-in demuxers. My "-correct-pts" patch is not directly related; it improves video pts handling in mplayer, but I haven't fixed anything in mencoder so far.
Hi, On Friday 07 July 2006 17:22, Evgeniy Stepanov wrote:
Setting these fields should be sufficient: Name, FontName, FontSize, ScaleX/Y, BorderStyle, Alignment, Outline, Colors and Margins.
Thanks, is there any document about these field? I can find some document about ass command on Sourceforge, but there is nothing about the Style part. and BTW, there is a small suggestion, in the ass_fontconfig.c FcConfigSubstitute(priv->config, pat, FcMatchFont); is called to setup the substitution, But when I match the font "Serif", it does not give me the same result as "fc-match", change FcMatchFont to FcMatchPattern gives the correct result. -- Best Regards, LR
On Friday 07 July 2006 14:07, 陆然 wrote:
Thanks, is there any document about these field? I can find some document about ass command on Sourceforge, but there is nothing about the Style part.
Attached. There was a link at matroska.org but it is broken now. Read carefully, the format is a bit strange. For example, truth values for Bold and Italic are (-1), the only valid values for BorderStyle are 1 and 3, and so on.
and BTW, there is a small suggestion, in the ass_fontconfig.c
FcConfigSubstitute(priv->config, pat, FcMatchFont);
is called to setup the substitution, But when I match the font "Serif", it does not give me the same result as "fc-match", change FcMatchFont to FcMatchPattern gives the correct result.
Thanks, it helps.
Hi, On Friday 07 July 2006 22:57, Evgeniy Stepanov wrote:
On Friday 07 July 2006 14:07, 陆然 wrote:
Thanks, is there any document about these field? I can find some document about ass command on Sourceforge, but there is nothing about the Style part.
Attached. There was a link at matroska.org but it is broken now.
Read carefully, the format is a bit strange. For example, truth values for Bold and Italic are (-1), the only valid values for BorderStyle are 1 and 3, and so on.
Thanks, one more thing, in order to alloc style and event in demux_mov, alloc_style and alloc_event should be declared non-static. or should I put those functions in ass.c, like the ass_process_line for mkv? -- Best Regards, LR
On Friday 07 July 2006 19:33, 陆然 wrote:
Thanks, one more thing, in order to alloc style and event in demux_mov, alloc_style and alloc_event should be declared non-static. or should I put those functions in ass.c, like the ass_process_line for mkv?
I'll make them non-static before committing.
On Wed, 5 Jul 2006 10:43:16 +0800 陆然 <hephooey@gmail.com> wrote: [...]
Have you considered extend the ssa support to a general subtitle support, recently I did a little hack to make mplayer support embeded mp4 subtitles, at the moment it is based on the old subtitle engine, so all the font/color/position settings are ignore. Suppose I want to take advantage of the ssa engine, is there a easy way to do this with the current implementation?
You can found a sample of the mp4 file here: http://rapidshare.de/files/5737885/1Video_2Audio_2SUBs_timed_text_streams_.m...
File added to samples/MPEG-4/embedded_subs Ciao, Roberto
On Tue, Jul 04, 2006 at 08:01:00PM +0400, Evgeniy Stepanov wrote:
On Monday 03 July 2006 18:56, Dominik 'Rathann' Mierzejewski wrote:
I've been using this patch for a while now with little or no problems at all, so I'm going to apply 1-5 soon if nobody objects. Hopefully after some comments from Moritz on demux_mkv changes.
Can I get maintainer status and svn write access ? I'd like to apply it myself if possible.
No problem with me, what do the others say? Diego
Hi, On 7/5/06, Diego Biurrun <diego@biurrun.de> wrote:
On Tue, Jul 04, 2006 at 08:01:00PM +0400, Evgeniy Stepanov wrote:
On Monday 03 July 2006 18:56, Dominik 'Rathann' Mierzejewski wrote:
I've been using this patch for a while now with little or no problems at all, so I'm going to apply 1-5 soon if nobody objects. Hopefully after some comments from Moritz on demux_mkv changes.
Can I get maintainer status and svn write access ? I'd like to apply it myself if possible.
No problem with me, what do the others say?
I'm fine with giving Evgeniy Stepanov, off course as long as he follows the rules (in order not to piss Reimar off) and does a good job as maintaining his code. However, searching my mail archives shows that there's been only 2 patches of Evgeniy Stepanov that got committed so far. So I guess giving SVN write access to Evgeniy Stepanov isn't so obvious, but I imagine it's better to have someone who writes few fine code/bugfixes instead of lots of bloated code. Guillaume -- "If God had wanted us to vote, he would have given us candidates." - Jay Leno
On Wed, 2006-07-05 at 10:25 +0200, Guillaume POIRIER wrote:
I'm fine with giving Evgeniy Stepanov, off course as long as he follows the rules (in order not to piss Reimar off) and does a good job as maintaining his code.
Why do you mention Reimar specifically? And I think "following the rules" isn't the most relevant factor deciding whether someone should get write access.
However, searching my mail archives shows that there's been only 2 patches of Evgeniy Stepanov that got committed so far.
So I guess giving SVN write access to Evgeniy Stepanov isn't so obvious, but I imagine it's better to have someone who writes few fine code/bugfixes instead of lots of bloated code.
If he's given write access primarily for the purpose of maintaining the SSA code he wrote then it shouldn't matter too much what other patches he's written.
Hello, On Wed, Jul 05, 2006 at 12:49:38PM +0300, Uoti Urpala wrote:
On Wed, 2006-07-05 at 10:25 +0200, Guillaume POIRIER wrote:
I'm fine with giving Evgeniy Stepanov, off course as long as he follows the rules (in order not to piss Reimar off) and does a good job as maintaining his code.
Why do you mention Reimar specifically?
I guess because I'm usually the first if not the only one to complain :-)
And I think "following the rules" isn't the most relevant factor deciding whether someone should get write access.
Being able to work with others IMHO is though. And ignoring other people's opinions (as among other things contained in the rules) is not helpful for that. But I don't think there will be a problem, after all even my bigger and optional suggestions were implemented without even a comaplaint ;-) Nevertheless I'm not exactly for discussing giving SVN write access in public, in the (probably unlikely) event that someone has objections it makes things a bit... uncomfortable.
If he's given write access primarily for the purpose of maintaining the SSA code he wrote then it shouldn't matter too much what other patches he's written.
No, though it can be used as an indication for the style, for the probability that the will just disappear when "the job is done" etc. Not that I think it really matters in this case. Greetings, Reimar Doeffinger
Hi, On 7/5/06, Reimar Doeffinger <Reimar.Doeffinger@stud.uni-karlsruhe.de> wrote:
Hello, On Wed, Jul 05, 2006 at 12:49:38PM +0300, Uoti Urpala wrote:
On Wed, 2006-07-05 at 10:25 +0200, Guillaume POIRIER wrote:
I'm fine with giving Evgeniy Stepanov, off course as long as he follows the rules (in order not to piss Reimar off) and does a good job as maintaining his code.
Why do you mention Reimar specifically?
I guess because I'm usually the first if not the only one to complain :-)
Yep, that's what I meant. I also forgot to add a smiley! :-) /me should get some rest... Guillaume -- "If God had wanted us to vote, he would have given us candidates." - Jay Leno
On Wed, Jul 05, 2006 at 12:41:16PM +0200, Reimar Doeffinger wrote:
On Wed, Jul 05, 2006 at 12:49:38PM +0300, Uoti Urpala wrote:
On Wed, 2006-07-05 at 10:25 +0200, Guillaume POIRIER wrote:
I'm fine with giving Evgeniy Stepanov, off course as long as he follows the rules (in order not to piss Reimar off) and does a good job as maintaining his code.
And I think "following the rules" isn't the most relevant factor deciding whether someone should get write access.
Being able to work with others IMHO is though. And ignoring other people's opinions (as among other things contained in the rules) is not helpful for that. But I don't think there will be a problem, after all even my bigger and optional suggestions were implemented without even a comaplaint ;-) Nevertheless I'm not exactly for discussing giving SVN write access in public, in the (probably unlikely) event that someone has objections it makes things a bit... uncomfortable.
True, but giving Evgeniy write access has been a recurring topic on IRC... Evgeniy, send me login name and password in private, I'll create an account for you. Wait a few more days in case some objections come up, but I doubt there will be any. Diego
Hey, On Monday 03 July 2006 16:56, Dominik 'Rathann' Mierzejewski wrote:
I've been using this patch for a while now with little or no problems at all, so I'm going to apply 1-5 soon if nobody objects. Hopefully after some comments from Moritz on demux_mkv changes.
The demux_mkv changes look very good. Just a minor nitpick: If the Matroska file contains attached fonts but no actual data (meaning just the name and/or MIME type, but not the actual font data, data == NULL || data_size == 0) then such an entry should be discarded and not handed over to ass_process_font. Yeah, such files would be broken, but that's no reason to have them break mplayer, too ;) Mosu -- If Darl McBride was in charge, he'd probably make marriage unconstitutional too, since clearly it de-emphasizes the commercial nature of normal human interaction, and probably is a major impediment to the commercial growth of prostitution. - Linus Torvalds
On Sun, 2006-06-25 at 14:45 +0200, seeboo wrote:
Somehow I can't apply the latest version of the Patch.
You shouldn't have sent over 1000 lines of post most of which is nothing but errors from all files failing because you specified a stupid -p value. The failure with the correct -p is because you either don't really have the latest version of the patch or are trying to apply it on sources that do not match latest svn.
On Jun 14, 2006, at 5:09 AM, Evgeniy Stepanov wrote:
Several bugfixes.
Btw, since the previous patch all ass-specific stuff (rendering and processing) is done under "if (ass_enabled)". This means that not giving -ass option disables all possible bugs in my code (except for compilation errors, of course). <mplayer-ass.patch.bz2>
This crashes on files that don't have a Formats line after Styles. The defaults seem to be Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, TertiaryColour, BackColour, Bold, Italic, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, AlphaLevel, Encoding for SSA and Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding for ASS. Also, http://astrange.ithinksw.net/KamichuRadio-America.mkv messes up on every multi-line subtitle.
I think the patch could be applied soon and further fixes done in svn. One thing I'd personally like to change before applying it is the indentation style of new files. MPlayer has traditionally added new files using whatever indentation style the author preferred, but as maintainers of files change over time and other people usually have to at least read the files I don't particularly like the end result. My idea of the most agreeable common style is basically that produced by "indent -kr", with the only significant difference from the current files being an indentation level of 4 spaces instead of the current 8. If you're going to keep working on the files and strongly prefer 8 spaces I don't have major objections against that (I don't find the files hard to read), but would prefer a 4-space indent. Some more comments about the latest version: +static struct vf_priv_s { + int outh, outw; + + unsigned int outfmt; + + // 1 = auto-added filter: insert only if chain does not support EOSD already + // 0 = insert always + int auto_insert; + + ass_instance_t* ass_priv; + + unsigned char* planes[3]; + unsigned char* dirty_rows; +} vf_priv_dflt = { + 0, 0, + + 0, + + 0, + + NULL, + + {0, 0, 0}, + 0 +}; You could just rely on the default initialization to 0 for static variables, and I think it would be clearer in this case. Various pointers in this data structure are filled with malloced values and apparently leaked when playing multiple files. +#define skip(x) if (*p == (x)) ++p; else { return p; } I think it would be nice to print a diagnostic (at least at verbose level, maybe higher) if some parts are skipped because of parsing errors, especially as not quite all features are yet supported. I had some subtitles using the 6-argument version of \move which became completely invisible due to clipping when the move was ignored, now there was no indication that anything might be missing. The specs I have say "Any style modifier followed by no recognizable parameter resets to the default". This doesn't seem to be handled correctly. I have subtitles that use {\c} to reset to the default color, now the displayed line is cut at that location. +void ass_face_cache_done(); +void ass_glyph_cache_done(); +ass_instance_t* ass_init(); +ass_image_t* ass_end_frame(); // returns linked list of images to render +ass_track_t* ass_new_track(); Old-style declarations, should have "void" as parameter list.
On Sunday 18 June 2006 22:01, Uoti Urpala wrote:
I think the patch could be applied soon and further fixes done in svn. One thing I'd personally like to change before applying it is the indentation style of new files. MPlayer has traditionally added new files using whatever indentation style the author preferred, but as maintainers of files change over time and other people usually have to at least read the files I don't particularly like the end result. My idea of the most agreeable common style is basically that produced by "indent -kr", with the only significant difference from the current files being an indentation level of 4 spaces instead of the current 8. If you're going to keep working on the files and strongly prefer 8 spaces I don't have major objections against that (I don't find the files hard to read), but would prefer a 4-space indent.
These files have 1 tab indentation, not 8 spaces. Why don't you set tab size to 4 ? Yes, I'm going to maintain them for some time.
+#define skip(x) if (*p == (x)) ++p; else { return p; }
I think it would be nice to print a diagnostic (at least at verbose level, maybe higher) if some parts are skipped because of parsing errors, especially as not quite all features are yet supported. I had some subtitles using the 6-argument version of \move which became completely invisible due to clipping when the move was ignored, now there was no indication that anything might be missing.
{} quite often is used for comments. Printing them at verbose level may be ok, but not higher.
On Sun, 2006-06-18 at 22:35 +0400, Evgeniy Stepanov wrote:
If you're going to keep working on the files and strongly prefer 8 spaces I don't have major objections against that (I don't find the files hard to read), but would prefer a 4-space indent.
These files have 1 tab indentation, not 8 spaces. Why don't you set tab size to 4 ? Yes, I'm going to maintain them for some time.
The problem is not so much the particular amount of indentation but the fact that all the files use different settings and styles, and often after a while when several people have worked on the files there are different styles within a file too. Setting the editor to use nonstandard tab sizes for some files would only make it worse. Anyway if you want to use tabs I won't insist on it.
I think it would be nice to print a diagnostic (at least at verbose level, maybe higher) if some parts are skipped because of parsing errors, especially as not quite all features are yet supported. I had
{} quite often is used for comments. Printing them at verbose level may be ok, but not higher.
Maybe {}, but what about after '\'?
On Sun, 18 Jun 2006 21:01:30 +0300 Uoti Urpala <uoti.urpala@pp1.inet.fi> wrote:
I think the patch could be applied soon and further fixes done in svn. One thing I'd personally like to change before applying it is the indentation style of new files. MPlayer has traditionally added new files using whatever indentation style the author preferred, but as maintainers of files change over time and other people usually have to at least read the files I don't particularly like the end result. My [...]
I don't agree with this, I like current policy and I don't see any reason to change it. Especially if the author is willing to mantain the code after it's committed. Ciao, Roberto
participants (27)
-
Alban Bedel -
Alexander Strange -
Attila Kinali -
David Lamparter -
Diego Biurrun -
Dominik 'Rathann' Mierzejewski -
Ergzay -
Evgeniy Stepanov -
Evgeniy Stepanov -
Guillaume POIRIER -
Hervé W. -
Jan Knutar -
Kridsada Krakronkul -
Lu Ran -
Moritz Bunkus -
Nico Sabbi -
Piotr -
Piotr Stachura
-
Reimar Doeffinger -
Reimar Döffinger
-
Rich Felker -
Robert Swain -
Roberto Togni -
seeboo -
Uoti Urpala -
Vladimir Mosgalin -
陆然