Re: [MPlayer-dev-eng] [PATCH] Fix spelling errors found by lintian.
- Date: Mon, 13 Jul 2015 04:22:07 -0400
- From: Miguel A. Colón Vélez <debian.micove@xxxxxxxxx>
- Subject: Re: [MPlayer-dev-eng] [PATCH] Fix spelling errors found by lintian.
> 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.
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"
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);
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
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
MPlayer-dev-eng mailing list