[MPlayer-cvslog] r27081 - trunk/libmpdemux/demuxer.c

[Michael Niedermayer]

On Mon, Jun 16, 2008 at 12:31:05PM +0200, diego wrote:
> Author: diego
> Date: Mon Jun 16 12:31:05 2008
> New Revision: 27081
> 
> Log:
> cosmetics: Consistently format all if, for, while constructs.

Iam happy to see someone try to properly cleanup mplayers code, assuming
the respective maintainers agreed to it ...
Some comments below though.

[...]
> -    if(demuxer->a_streams[id]){
> +    if (demuxer->a_streams[id])
>          mp_msg(MSGT_DEMUXER,MSGL_WARN,MSGTR_AudioStreamRedefined,id);
> -    } else {
> +    else {

While the {} are useless here i tend to think that keeping them would be
slightly preferable, the reason is adding code.

 if(X){
    A;
+   B;
 }else {

vs.

-if(X)
+if(X){
    A;
+   B;
-else {
+}else {

Above is clearly more readable

for normal
if(X){
    A;
}

the closing } costs a line but with if/else it does not.



>          sh_audio_t *sh = calloc(1, sizeof(sh_audio_t));
>          mp_msg(MSGT_DEMUXER,MSGL_V,MSGTR_FoundAudioStream,id);
>          demuxer->a_streams[id] = sh;
> @@ -300,15 +300,14 @@ void free_sh_audio(demuxer_t *demuxer, i
>  
>  sh_video_t* new_sh_video_vid(demuxer_t *demuxer, int id, int vid)
>  {

> -    if(id > MAX_V_STREAMS-1 || id < 0)
> -    {
> +    if (id > MAX_V_STREAMS - 1 || id < 0) {

its off topic but these can be simplified by using unsigned id.

[...]
> +      if (demux->reference_clock != MP_NOPTS_VALUE) {
> +        if ((p->pts != MP_NOPTS_VALUE)
> +            && (p->pts > demux->reference_clock)
>              && (ds->packs < MAX_ACUMULATED_PACKETS)) {

the first check could be vertically aligned with the following and the () are
superflous

[...]
> -    if(ds->buffer_pos>=ds->buffer_size){
> +    if (ds->buffer_pos >= ds->buffer_size) {
[...]
> -    if(ds->buffer_pos>=ds->buffer_size){
> +    if(ds->buffer_pos >= ds->buffer_size){

does not look consistant to me


[...]
> -  if(!vd) {
> -    if(as) free_stream(as);
> -    if(ss) free_stream(ss);
> +  if (!vd) {
> +    if (as)
> +        free_stream(as);
> +    if (ss)
> +        free_stream(ss);

I belive the original was more readable, also maybe a NULL check should
rather be moved into free_stream()


[...]
> -    if (ans < 0) ans = 0;
> -    if (ans > 100) ans = 100;
> +    if (ans < 0)
> +        ans = 0;
> +    if (ans > 100)
> +        ans = 100;

av_clip() 

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-cvslog/attachments/20080616/3015845b/attachment.pgp>