From 337e0c62f894722e9c268b14d02a85b84c96024d Mon Sep 17 00:00:00 2001 From: pukkandan Date: Sat, 29 May 2021 01:39:07 +0530 Subject: [PATCH] [embedthumbnail] Correctly escape filename Closes #352 The approach in [1] is faulty as can be seen in the test cases 1. https://github.com/ytdl-org/youtube-dl/commit/bff857a8af696e701482208617bf0b7564951326 --- .gitignore | 85 +++++++++--------- test/test_postprocessors.py | 27 +++++- .../thumbnails/foo %d bar/foo_%d.webp | Bin 0 -> 3928 bytes yt_dlp/postprocessor/embedthumbnail.py | 2 +- yt_dlp/postprocessor/ffmpeg.py | 17 ++-- 5 files changed, 75 insertions(+), 56 deletions(-) create mode 100644 test/testdata/thumbnails/foo %d bar/foo_%d.webp diff --git a/.gitignore b/.gitignore index a2484b752..b6431b766 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,46 @@ +# Config +*.conf +*.spec +cookies +cookies.txt + +# Downloaded +*.srt +*.ttml +*.sbv +*.vtt +*.flv +*.mp4 +*.m4a +*.m4v +*.mp3 +*.3gp +*.webm +*.wav +*.ape +*.mkv +*.swf +*.part +*.part-* +*.ytdl +*.dump +*.frag +*.frag.urls +*.aria2 +*.swp +*.ogg +*.opus +*.info.json +*.live_chat.json +*.jpg +*.png +*.webp +*.annotations.xml +*.description + +# Allow config/media files in testdata +!test/testdata/** + # Python *.pyc *.pyo @@ -43,48 +86,6 @@ README.txt yt-dlp.zip *.exe -# Downloaded -*.srt -*.ttml -*.sbv -*.vtt -*.flv -*.mp4 -*.m4a -*.m4v -*.mp3 -*.3gp -*.webm -*.wav -*.ape -*.mkv -*.swf -*.part -*.part-* -*.ytdl -*.dump -*.frag -*.frag.urls -*.aria2 -*.swp -*.ogg -*.opus -*.info.json -*.live_chat.json -*.jpg -*.png -*.webp -*.annotations.xml -*.description - -# Config -*.conf -*.spec -cookies -cookies.txt - - - # Text Editor / IDE .idea *.iml diff --git a/test/test_postprocessors.py b/test/test_postprocessors.py index 7574a0b95..868bb25f9 100644 --- a/test/test_postprocessors.py +++ b/test/test_postprocessors.py @@ -8,7 +8,11 @@ import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -from yt_dlp.postprocessor import MetadataFromFieldPP, MetadataFromTitlePP +from yt_dlp.postprocessor import ( + FFmpegThumbnailsConvertorPP, + MetadataFromFieldPP, + MetadataFromTitlePP, +) class TestMetadataFromField(unittest.TestCase): @@ -30,3 +34,24 @@ class TestMetadataFromTitle(unittest.TestCase): def test_format_to_regex(self): pp = MetadataFromTitlePP(None, '%(title)s - %(artist)s') self.assertEqual(pp._titleregex, r'(?P.+)\ \-\ (?P<artist>.+)') + + +class TestConvertThumbnail(unittest.TestCase): + def test_escaping(self): + pp = FFmpegThumbnailsConvertorPP() + if not pp.available: + print('Skipping: ffmpeg not found') + return + + file = 'test/testdata/thumbnails/foo %d bar/foo_%d.{}' + tests = (('webp', 'png'), ('png', 'jpg')) + + for inp, out in tests: + out_file = file.format(out) + if os.path.exists(out_file): + os.remove(out_file) + pp.convert_thumbnail(file.format(inp), out) + assert os.path.exists(out_file) + + for _, out in tests: + os.remove(file.format(out)) diff --git a/test/testdata/thumbnails/foo %d bar/foo_%d.webp b/test/testdata/thumbnails/foo %d bar/foo_%d.webp new file mode 100644 index 0000000000000000000000000000000000000000..d64d0839f054071849aa12f194b8b20b19e6bb59 GIT binary patch literal 3928 zcmb`~_dgVX;|B0EvWr7mA)$~Jm7J}!$vn<Fd#`L~T_VZ8%XUaE>+C%bAtz*&5!t#N zM><8u`F>xo&+GFae4byP=lv%<W_sG%CaeH}m6oQ7rOD%4;Q!xKPJkjRiB+nHRH{hb zw}zq^;oCyL^%xr189n^h!>E7)ILKgM{C&GlLaE?Cfjmla3>)I?=d&Zkv^~-GLWfe{ zK>u^3wjFzU%IaXRpvHx8A_<<Y{@M5G^Mb>BxfTx=;c=8V{e2M1TL%=v@KU&^381D# z$)%j_5oUzXGr*LID8MsK6PkZ&CmB!FDx)bE3YYB{I{g9U08+2VWr#`!!keagUZ#(U ziG}Szb^2>YOQ;KpA9|)^`9H=~U$>r3SB#uq^T?wlDNZFPH5vl8CWvd8)3MkjQS^KW z`|(CDZ$n}8?|rjF>cov{+=^mRXJv(;?@bBz<LvR|u#}ATQDRsVm1Ui$qynqgbeDnT zF#dBZvOR#?ao+h#7nOyf!AC*Bu0R7oE)@Tr-BD@hl{<xN&I^-~w5mOk$DyxT(kIi$ zPAqqXt3WuO4_;#2*fK`7Wu|%RluS*T2mJMKL2_lA-WN-m*z^*bNx?)Z2&KHyV4*Sd zo_+bRakFl&LPR8OTmN&K8g(J%v=T7;Hz%EG{>N&Lh_M*==DUGHe%aB(L*1yCoez9M zyGr;+(Ovgt@q6(?+CVOdR%MzzUQ6ZbYOZ0KCa|hEZ^@ZpfsJt0Uk#jaKDqpsgU|WP zxIX1V?Lj&^#(PQAykx~TRh6}TPc{gld91w>tRl2y3(jryT8-n)Pht{#6ZocJx9t6d z!LK{ltSgrSB7<uwJVroKl(XWZ`}#4gXY&#ut6F!r;XtPR!8eIYlkxOV>RZtqsH5Ez zP-JpJu4^L2#3yrl49UB7v8Yav>N(Tx60!j`{!Oom8QZ2~2ruy@j(q!%%dmp~E2RAb zP*%AC{|!CP^?G(;0#}{Lx(ALpXO<FAqmhE)vk8Bzv01z(AtomAn(FtBFm6Y}LuY6h zJT=%##phLk0~smnj!fc|$s73)@dFauWEIqR7e8c+3c3P~b>shmm(<c{qi=CIHlV%q zcf$}%ctqLQAJ}ZEv$u4sPkgXycBU%{8S56|k7jsLF?RJ@?80YY0X;Z#id2+))6eHF zw~S4@nK2}8-c#vgMHa%oIBlx)^T9#Ae8j1sv2uLP9jb(dnw<hLvRj)<G830V%Cb*h zb>2G9;&=thDR-GH0O6ts6P8%#ShAAXW^znkqhw0nIrZp5%o`b!ZuV9+)E;a|@Z}y= zu(%%zpEXRx)ZJp+bZ{N(R7k*1A*p~*CG{iSv9v6Yp82v-mr7cqvV9ahKm#JZS0=ot zDlO!LcOd+80*d=7V`Zbe`T07*4#q{aS5HdUZ_a}HtG;Rf{ljdko$$oRzh_UCezmWO zZ*nc-G}vl@cQiJDR<uv|i-um!LyeW7U;L@}5Rzz{TDdcTv=&R^RS}1LlQPhwKmfS8 zu&>CpdiF5c+roHeq{VHvAvme+dm4x<qP7HM?Y#VZGnB#ka0^GS2f5Gcj^B@R2bnJ7 zD$d&5*yG|%f{Yo|Q`mJ+D@&lsv==%nO~D7?O0x0O;y^9$gwyyZ@ro+a5-@kESmJ3L zbq}TcMlw$F&U^o*bV3`CqKWOJuQW_Y;-z@jK|Y(L%|j2H=1%qi?LA`KxpH&0!1IKX zHS7CL#Wo~+^6w<FZ~j9zVr&sUZ8#xnJJh1uw)|~OSIW1$Xb(dHue9(C%QTkGfQRW% z4sBBe2AA`TM~5fca$WKl5H^+M1?LaeA*Y_Mp<0ud>JQl#<!>LrdAWW$i`&aRi}-7( z)&e{#56KI6QjOr9z1}2LxqeADdiZzlj>$plMm-F}YCYS0ykxa8Q1#mxDpd{^rWd#) z;qxawT<b&NiaLDlcRrCnw%^!klcncR2s+a6?y}W(#7(4>1T$wDG=#Z8<Kn9{!)6tU zc)%Ee?M_FI{yZ7(kJ$$ujLYjS7w}(&lX~e0=%ehBy)82HB&|GLcFgt&0|}mXj6W5@ z=rE*^Co)j{!0OcIrk@))4OzV{YCOlYp~b8itg^CDrCKX(w0L%AZ~$aDA73L5_Yh|# zOnAbFn*s=f%_=LQuo&OBpSa*_!ggvJUbU+o#*S{|foe=vOILb<cP_s}Tvp7HYEE?r z;*nM$&g+VJ#O`9hZxRwi_G7Sxf%v!;S;{>+yXlz75~-PRlt`i00>Ugew=7R}_Zx%C zpT|rizqBo)P#6_>$*F%v?-d7UoldpQYO`zhejAM0(a%TS(K0C{`;jEuD8a%^i;E13 z{Mr1qQg@SC4@vH@9PUFw0|)GEH?LR(>kFP87S@rKgLb|R2*=_B7z88K<`=Fg*jS7m zZh;!+3r4c7>FirW5|T#&e*u;fL|3-{uNd<6*B`KCT9W94H$8C!h~c)1o82-U22zSr zB7Du!8)SJvlz>XhwjK7<Af$qiCg*B{PHRlm>AUWhB-c&2?SYGFS$)AbWVi9NeGC66 z&W7Fz9Q8znbl#k1O5D6`vy6Tg6D4a{AQj8OYgN9TL?`iVgp(~ZnbY6b;7GK4O13ji z1zhQ6-9B+TyP2FRlEcTNFc9#*_@f18(2hgrC`KZts{ZLJ1g?1qd%RS!)&)<MytHKf z>I&&y_|=*9Md)i!>(jZAgz7Sqyy$)gPg-N@=|5qID2}H|^uB@3enIKc;0*Ng4o!dJ zWQEy!^t!68U32z>ldxR+mj_;&UPI!MpFP}YBBD5sIyegIymbctib)QR(o%9O!(?Td zK6ORqa2|mB(FFU{Hz&t#;4fTan)(J!ISByq{23BphZdQAu^Ap7>1TI(DUOs*k@q+( z)}()Zg3@>qX}O?{QdXx;bWV>Wv+UkDz2Uj=$ga+VdA?hN`{qQU`O8tW!(+A1Cdrrf zQ!~t~J{wv<HXbT&uv^8j5?O&Ps|0D#88xpMnLGHQegM+GRhONhOB7Xkpylbg+4PMT zxT#gF66;`XtTU3*`>C7tD^IN%cu8z=l>4zz7{@1^p?j2FTur<hjguC>KBqPxTMLee zZ?YMmSyr&6E?Ib-xa!wp7JjDbaSycO8S23Cgqmg7ut!YPis}WOqV^B>W1|iXs-4HD ziZt)lXm_08nQU+i86xctG7rEl)lZ6y(5x4+pDTTkL#Jh_@<`7<Xl*i5@t<LCqkIc& z*)CA9TZ##=WzWst3#Id32#?jfyB(ccCB&ch+pU#>AUOYwcV~g3X!J37qr==x8LBMG zs3Qm@2KATnz@or5y!U*>>I3sKRitt-Nm4{%BA8*v<d@c%iJMWESM~^RSJB=!>SO1I z-TBkal#y}l{wy@%JLk_t$eXc_7<x~}NAa?($gJQ_(;t30LDycR&&CYO@nH}gK)E-I zNs=rTW|Ad-n2%!=F6ST-rlL!@wc=U|p@wAB6+7AWN2%K-gN5gJDCgVS!tT?mTqry{ z>B`4C-bw1=o<gc{xtWwmMxGY$E_P!o*u^9l)u*?QG@~323&zpOU&X`F{^UIMXD>QT zOG=~n5vf9O?%PIK%RDFGQ$)vm`smN{*N}#qk5ATfnpFC*{9&a&MxS-w*4WP?h0#=u zKwOQsYe=OBy2$fTQVJS5yrWDU#7uADW)i$p58A`>Zi}|LVP-*s0Ot0A0qx+$ix<^b z*yefreyF;#<-WA|{t`&J!K-*ZJ#s-_aVI^lQ~byImxuQf2Xm09S+v=3ux_Genm64m zCSA!QyfR(!-dd*)jE^zcaLF;SQU|#28br_KBUw}w`_jVO_kMmUC-e0IumQgMR2D(( zXapmum}1d2Y+r)9M{o?>leMG1PJfe*R^9HRd7v}39@H9@iQ51$tWN$V#V~E3zDl+s z+`U!n+_m?nCBY@E%UlGdNY{fU-0!#4qBbvEY~$2ER?OqPcw<A02)8HVOR9{Hj6-r| zn;EM;wt@Ham!Yt6Ge`y353fGxcA3VD7bOAnTu;W}rFhW2)+U9UuBdl3HTjo5x+e0~ zVKYi|tYzN)RtM%qg9<)&akf(WYo5)*hL#rQTqy#(%u>>%4s+4HpC>3a%*UH99qJk@ z1Dk4^boGJ3AE6f`rtPwh#mb(np{kAw7Jo<~)-v#2?KuF2)@*Wli4fN8()8)7yq?<} zgnBk};ECdiD0CWAcwaf^y!^_fnJ-Y|>EG(VM}%~{N7fHb6d#K=3}7^k3YC}PvAcqZ zU_b^qkmj1{?b+4>U;#~+kx_b*65p><r9^UI+iPwOL6o=vckS!ebEwy)ROeaM#)efw zM=pN`EP`|@cvwUMh=W#*ud}REZTW!+twM60i2EG+f**`lXWQkbVzQy5jBC*emOmPc zjE7Q8SZI__pVV42>x}GJ?dp4i_=%eiu*JwNvt?$*Hz5zS2*Izit$A^lTYd}grzO;` zO|OQ+po8&Lv91}gUJFyD2oRiUVnufu)6QVbZjJo(i)pX$LW0w4LAgSzMpjJ9g@Ng# zmuDjGdn&6v4yUb9FaIxRDC~Ns1y>j$eH0n~N3%telVR))|K*a%;sB@6DNj(raSBi^ z{n&TFPhp&U-TMjm?O|^cQzI1Eb2+rjgEe*9TZB|CF66SI_09)tqv95Sz%Hya^-egh zTm18|Rdqv`aVvB5PsJT`QiXdd6UHQjz#I`{TgD&C4?Q;>J!3-Wr$=Q!NgEAsYMrsZ zj*DFL`U7%Dyo+@b6#L#{e(1bdcy#o<Ak!%ZvRS2$@4Bi#P*oW-k|yK}FMfxfN#N1o z%gQvOTaG^?MpSU8am<|u$nluVdBR}G7p6Sq=lL01O1`EO=vUj(n<b5=MuN-)g|R~L z!r(^GWTxTOS1`k`&_-$@4Ti{ni1y6kfEVSUfvqxW&#n%>b=VU*)PUvC)t7R`*mkFq zjJs{iz-Rc<idRLQ8Of8EwXqdKy&uenc&d({;$GR$e;3m4Q%W`>CNt23{KZ!Tf;Kat bLpo5mJGW#CT`nX2$#Kl+y#GFf|MdR>k&2#! literal 0 HcmV?d00001 diff --git a/yt_dlp/postprocessor/embedthumbnail.py b/yt_dlp/postprocessor/embedthumbnail.py index f3eb7d96d..278a45eb6 100644 --- a/yt_dlp/postprocessor/embedthumbnail.py +++ b/yt_dlp/postprocessor/embedthumbnail.py @@ -70,7 +70,7 @@ def run(self, info): self.to_screen('There aren\'t any thumbnails to embed') return [], info - idx = next((-(i+1) for i, t in enumerate(info['thumbnails'][::-1]) if t.get('filepath')), None) + idx = next((-i for i, t in enumerate(info['thumbnails'][::-1], 1) if t.get('filepath')), None) if idx is None: self.to_screen('There are no thumbnails on disk') return [], info diff --git a/yt_dlp/postprocessor/ffmpeg.py b/yt_dlp/postprocessor/ffmpeg.py index ea728be37..d9f816b04 100644 --- a/yt_dlp/postprocessor/ffmpeg.py +++ b/yt_dlp/postprocessor/ffmpeg.py @@ -853,19 +853,12 @@ def _options(target_ext): return [] def convert_thumbnail(self, thumbnail_filename, target_ext): - # NB: % is supposed to be escaped with %% but this does not work - # for input files so working around with standard substitution - escaped_thumbnail_filename = thumbnail_filename.replace('%', '#') - os.rename(encodeFilename(thumbnail_filename), encodeFilename(escaped_thumbnail_filename)) - escaped_thumbnail_conv_filename = replace_extension(escaped_thumbnail_filename, target_ext) - - self.to_screen('Converting thumbnail "%s" to %s' % (escaped_thumbnail_filename, target_ext)) - self.run_ffmpeg(escaped_thumbnail_filename, escaped_thumbnail_conv_filename, self._options(target_ext)) - - # Rename back to unescaped thumbnail_conv_filename = replace_extension(thumbnail_filename, target_ext) - os.rename(encodeFilename(escaped_thumbnail_filename), encodeFilename(thumbnail_filename)) - os.rename(encodeFilename(escaped_thumbnail_conv_filename), encodeFilename(thumbnail_conv_filename)) + + self.to_screen('Converting thumbnail "%s" to %s' % (thumbnail_filename, target_ext)) + self.real_run_ffmpeg( + [(thumbnail_filename, ['-f', 'image2', '-pattern_type', 'none'])], + [(thumbnail_conv_filename.replace('%', '%%'), self._options(target_ext))]) return thumbnail_conv_filename def run(self, info):