[PATCH] Fix spelling errors found by lintian.
Hello: While packaging MPlayer for Debian, lintian found the following issues: I: mencoder: spelling-error-in-binary usr/bin/mencoder Psychadelic Psychedelic I: mencoder: spelling-error-in-binary usr/bin/mencoder bandwith bandwidth I: mplayer-gui: spelling-error-in-binary usr/bin/gmplayer Psychadelic Psychedelic I: mplayer-gui: spelling-error-in-binary usr/bin/gmplayer bandwith bandwidth I: mplayer: spelling-error-in-binary usr/bin/mplayer Psychadelic Psychedelic I: mplayer: spelling-error-in-binary usr/bin/mplayer bandwith bandwidth The following trivial patch fixes the issue and silences lintian. Note: id3v2.3.0 and earlier have the Psychadelic typo but id3v2.4.0 fixes it. http://id3.org/id3v2.4.0-frames Cheers, Miguel
Hi! On 2015-07-12 16:27 -0400, Miguel A. Colón Vélez wrote:
Hello:
While packaging MPlayer for Debian, lintian found the following issues:
Nice, some packaging efforts. I did some experiments for that too last year, but did not have much time to work on it. I will try to dig up the findings from that time.
I: mencoder: spelling-error-in-binary usr/bin/mencoder Psychadelic Psychedelic I: mencoder: spelling-error-in-binary usr/bin/mencoder bandwith bandwidth I: mplayer-gui: spelling-error-in-binary usr/bin/gmplayer Psychadelic Psychedelic I: mplayer-gui: spelling-error-in-binary usr/bin/gmplayer bandwith bandwidth I: mplayer: spelling-error-in-binary usr/bin/mplayer Psychadelic Psychedelic I: mplayer: spelling-error-in-binary usr/bin/mplayer bandwith bandwidth
The following trivial patch fixes the issue and silences lintian.
Note: id3v2.3.0 and earlier have the Psychadelic typo but id3v2.4.0 fixes it. http://id3.org/id3v2.4.0-frames
First of all both of the changes, are in files that were copied from other projects. For the id3v2, I am not sure about the implications and if we should really change it. For the "bandwith" typo in the comments/error message, I would really rather not change it. It would make comparison to upstream harder with not much gain. This is a kind of superficial response, because I do not have deep knowledge about SDP nor about the usage of the id3v2 genre table in MPlayer/MEncoder. I noticed that the last time the typo was changed, it was reverted to the current state. Alexander
Description: Fix spelling errors found by lintian. I: mencoder: spelling-error-in-binary usr/bin/mencoder Psychadelic Psychedelic I: mencoder: spelling-error-in-binary usr/bin/mencoder bandwith bandwidth I: mplayer-gui: spelling-error-in-binary usr/bin/gmplayer Psychadelic Psychedelic I: mplayer-gui: spelling-error-in-binary usr/bin/gmplayer bandwith bandwidth I: mplayer: spelling-error-in-binary usr/bin/mplayer Psychadelic Psychedelic I: mplayer: spelling-error-in-binary usr/bin/mplayer bandwith bandwidth . Note: id3v2.3.0 and earlier have the Psychadelic typo but id3v2.4.0 fixes it. http://id3.org/id3v2.4.0-frames Author: Miguel A. Colón Vélez <debian.micove@gmail.com> Forwarded: yes Last-Update: 2015-07-12
--- mplayer-1.1.1+svn37424.orig/libmpdemux/genres.h +++ mplayer-1.1.1+svn37424/libmpdemux/genres.h @@ -99,7 +99,7 @@ const char * const genres[] = { "Native American", /* 64 */ "Cabaret", /* 65 */ "New Wave", /* 66 */ -"Psychadelic", /* 67 */ /* typo taken from the id3v2spec*/ +"Psychedelic", /* 67 */ "Rave", /* 68 */ "Show Tunes", /* 69 */ "Trailer", /* 70 */ --- mplayer-1.1.1+svn37424.orig/stream/freesdp/common.h +++ mplayer-1.1.1+svn37424/stream/freesdp/common.h @@ -154,7 +154,7 @@ typedef enum } fsdp_address_type_t;
/** - * @short Type of bandwith modifiers + * @short Type of bandwidth modifiers * * Bandwidth modifiers specify the meaning of the bandwidth * value. Initially "Conference Total" and "Application Specific" are --- mplayer-1.1.1+svn37424.orig/stream/freesdp/errorlist.c +++ mplayer-1.1.1+svn37424/stream/freesdp/errorlist.c @@ -41,7 +41,7 @@ const char *fsdp_error_t_s[] = { "Syntax error in connection item", /** FSDPE_INVALID_CONNECTION **/ "Unrecognized address type in connection item", /** FSDPE_INVALID_CONNECTION_ADDRTYPE **/ "Unrecognized network type in connection item", /** FSDPE_INVALID_CONNECTION_NETTYPE **/ - "Parse error in bandwith item", /** FSDPE_INVALID_BANDWIDTH **/ + "Parse error in bandwidth item", /** FSDPE_INVALID_BANDWIDTH **/ "No time period for the session", /** FSDPE_MISSING_TIME **/ "Parse error in time item", /** FSDPE_INVALID_TIME **/ "Parse error in repeat time item", /** FSDPE_INVALID_REPEAT **/ --- mplayer-1.1.1+svn37424.orig/stream/freesdp/parser.h +++ mplayer-1.1.1+svn37424/stream/freesdp/parser.h @@ -242,9 +242,9 @@ unsigned int fsdp_get_bw_modifier_count * @param index number of bandwidth modifier. * * @return global bandwidth modifier type. - * @retval FSDP_BW_MOD_TYPE_UNDEFINED if no global bandwith modifier + * @retval FSDP_BW_MOD_TYPE_UNDEFINED if no global bandwidth modifier * type is defined or invalid index. - * @retval FSDP_BW_MOD_TYPE_UNKNOWN if an unknown bandwith modifier is + * @retval FSDP_BW_MOD_TYPE_UNKNOWN if an unknown bandwidth modifier is * specified or an invalid index is provided. In this case * fsdp_get_bw_modifer_type_unknown() can be called to get the * modifier as a character string.
Nice, some packaging efforts. I did some experiments for that too last year, but did not have much time to work on it. I will try to dig up the findings from that time.
I have a package for it working but it's waiting for the FFmpeg transition. http://anonscm.debian.org/cgit/pkg-multimedia/mplayer.git/tree/?h=master.exp... Not sure if it's the right place to ask but is there a reason for not including "libavutil/x86/asm.h" somewhere in the subversion? This FFmpeg file seems to be the only one needed for a successful compilation with a shared FFmpeg library. It seems to be a private header and therefore not included in Debian's or any other Linux distros (that I know of). It's used in: ./configure: header_check libavutil/x86/asm.h || die "libavutil/x86/asm.h header is required for shared FFmpeg" ./cpudetect.c:#include "libavutil/x86/asm.h" ./libmpcodecs/pullup.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_ass.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_decimate.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_divtc.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_filmdint.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_fspp.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_gradfun.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_ilpack.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_ivtc.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_noise.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_tfields.c:#include "libavutil/x86/asm.h" ./libmpcodecs/vf_yadif.c:#include "libavutil/x86/asm.h" ./libvo/aclib_template.c:#include "libavutil/x86/asm.h" I currently curl that file into the head revision to create a source since I don't need the complete FFmpeg source.
First of all both of the changes, are in files that were copied from other projects.
For the id3v2, I am not sure about the implications and if we should really change it.
For the "bandwith" typo in the comments/error message, I would really rather not change it. It would make comparison to upstream harder with not much gain.
This is a kind of superficial response, because I do not have deep knowledge about SDP nor about the usage of the id3v2 genre table in MPlayer/MEncoder. I noticed that the last time the typo was changed, it was reverted to the current state.
The genre is stored as a number between 0-255 in the id3 tags. It's only directly used in libmpdemux/demux_audio.c: g = stream_read_char(s); demux_info_add(demuxer,"Genre",genres[g]); The genres table seems to just translate that number into something the user would understand when displayed. The typo seems to have been left as is because as explained in the comment "typo taken from the id3v2spec". That comment was not part of the original id3edit source (from 2002) so I assumed that was the reason the typo was left unchanged and that is why I mentioned it was fixed in the latest id3v2spec. It was not mentioned as an error in the spec changes documents so it was most likely a simple typo that was never meant to be in previous versions of the spec. For freesdp I think only lintian cares about this one: "Parse error in bandwith item", /** FSDPE_INVALID_BANDWIDTH **/ the other ones where found by sed. Looking at the source this array is just another array to translate a number into text. It's also "dead code" since it seems to be only accessed by fsdp_strerror() but that function is never used. I thought that the changes were safe and since the last version from id3edit is from 2002 and freesdp is from 2006 they are most likely never getting fixed by their upstream. I will admit that these changes are rather pedantic/trivial since the only issue would be that if an user plays an mp3 with genre byte = 67 it would be displayed as Psychadelic if they decide to read the id3 tags. The freesdp part would only be an issue if future code decides to use the unused function which just outputs a human readable error message. I understand if this does not get applied. The other patch that I sent after this one about the manual pages is the more important of the two. Cheers, Miguel
On Mon, Jul 13, 2015 at 04:22:07AM -0400, Miguel A. Colón Vélez wrote:
Nice, some packaging efforts. I did some experiments for that too last year, but did not have much time to work on it. I will try to dig up the findings from that time.
I have a package for it working but it's waiting for the FFmpeg transition. http://anonscm.debian.org/cgit/pkg-multimedia/mplayer.git/tree/?h=master.exp...
Not sure if it's the right place to ask but is there a reason for not including "libavutil/x86/asm.h" somewhere in the subversion? This FFmpeg file seems to be the only one needed for a successful compilation with a shared FFmpeg library. It seems to be a private header and therefore not included in Debian's or any other Linux distros (that I know of). It's used in:
I do not mind that much, it's just that it would be effort to maintain. If you were to volunteer to from time to time update (or remind someone to update) it if it's changed in FFmpeg then I'd not even have a minor objection left :)
The genres table seems to just translate that number into something the user would understand when displayed. The typo seems to have been left as is because as explained in the comment "typo taken from the id3v2spec". That comment was not part of the original id3edit source (from 2002) so I assumed that was the reason the typo was left unchanged and that is why I mentioned it was fixed in the latest id3v2spec.
If the spec was updated, I guess it is fine to change. I don't know how relevant staying in sync with upstream is, probably not much. I'd be very, very slightly in favour of changing them just to shut up the warnings as we'll probably never again sync with upstream but if others have opinions otherwise I will concede.
On 2015-07-15 21:24 +0200, Reimar Döffinger wrote:
On Mon, Jul 13, 2015 at 04:22:07AM -0400, Miguel A. Colón Vélez wrote: [...]
The genres table seems to just translate that number into something the user would understand when displayed. The typo seems to have been left as is because as explained in the comment "typo taken from the id3v2spec". That comment was not part of the original id3edit source (from 2002) so I assumed that was the reason the typo was left unchanged and that is why I mentioned it was fixed in the latest id3v2spec.
If the spec was updated, I guess it is fine to change.
In principle I am fine with it too.
I don't know how relevant staying in sync with upstream is, probably not much. I'd be very, very slightly in favour of changing them just to shut up the warnings as we'll probably never again sync with upstream but if others have opinions otherwise I will concede.
My reasons for being unsure about it were: 1) I was not sure if MEncoder might be affected; now I am rather sure it doesn't affect on MEncoder 2) This is part of -identify output, thus it could cause problems for other programs parsing this output (e.g. using that output for grouping by genre, could end up with 2 instead of one groups for Psychedelic) I am not opposed to fixing the genre typo. In addition I would add a sentence in Changelog that mentions it. FWIW to comply with what is in common range to the current spec we would have to do these adjustments: 25: Eurotechno -> Euro-Techno 40: Alternative Rock -> AlternRock 67: Psychadelic -> Psychedelic 69: Show Tunes -> Showtunes I do *not* think we should change the others too. BTW I am wondering if there is a robust way to suppress false positives reported by lintian. Alexander P.S. If there are no more opinions on this I will just go with fixing the typo in genres.h .
Not sure if it's the right place to ask but is there a reason for not including "libavutil/x86/asm.h" somewhere in the subversion? This FFmpeg file seems to be the only one needed for a successful compilation with a shared FFmpeg library. It seems to be a private header and therefore not included in Debian's or any other Linux distros (that I know of). It's used in:
I do not mind that much, it's just that it would be effort to maintain. If you were to volunteer to from time to time update (or remind someone to update) it if it's changed in FFmpeg then I'd not even have a minor objection left :)
Currently I manually run a script that tells me "UPDATE ASM.H IN MPLAYER" whenever the files changes in the FFmpeg master branch. It would take about the same amount of effort to fix it in the packaging for Debian or update/ping someone in here. I don't mind helping for as long as I work in the Debian packaging which hopefully would be for a while. This should also help others since from a quick look: Arch Linux => just dumps FFmpeg 2.6.2 into their source Fedora => patches in a static asm.h Gentoo => Downloads asm.h from FFmpeg master. Gentoo also downloads libavcodec/allcodecs.c libavformat/allformats.c libavfilter/allfilters.c Which are always checked for but I think the information from those files is only used if using an internal copy of FFmpeg. Including it or not, either way is fine by me. In an on topic note. Hopefully the spelling typo patch gets included if there are no objections. The FFmpeg transition should begin today so hopefully a mplayer upload should follow soon. Cheers, Miguel
On 2015-07-20 09:21 -0400, Miguel A. Colón Vélez wrote: [...]
In an on topic note. Hopefully the spelling typo patch gets included if there are no objections. The FFmpeg transition should begin today so hopefully a mplayer upload should follow soon.
I split your patch and stripped of the comment changes for freesdp. Both patches committed. Hope those lintian warnings are gone for you now, Alexander
I split your patch and stripped of the comment changes for freesdp.
Both patches committed.
Hope those lintian warnings are gone for you now, Alexander
Thanks for applying the patch. I just compiled it and all the lintian issues that I reported are gone. The remaining ones are for the manual pages which have lines that are like parameter1:parameter2:parameter3:parameter4:parameter5:parameter6:parameter7:parameter8 man thinks it's just one huge word and can't word-wrap it if the resolution is low enough. I'm not sure of an elegant way to fix those and sticking a space in the middle is kind of ugly and misleading so I have no patch for those. Thanks for everything. - Miguel
participants (3)
-
Alexander Strasser -
Miguel A. Colón Vélez -
Reimar Döffinger