Re: wavwrite.m - how to commit updated function?

[Børge Strand-Bergesen]

Thank you for the feedback. I have edited the file and reattached it.

John, introducing "int24" as an fwrite/fread format would remove the
need for making a special case out of 24-bit .wav data. It wouldn't
surprise me if there is a way to call today's fwrite which does this
in a more efficient way. I just haven't used it enough to tell.

Jordi, I'd appreciate it if you could generate the properly formatted patch.

Børge

On Wed, Sep 5, 2012 at 11:48 PM, John W. Eaton <address@hidden> wrote:
> On  5-Sep-2012, Jordi Gutiérrez Hermoso wrote:
>
> | On 5 September 2012 17:10, Børge Strand-Bergesen <address@hidden> wrote:
> | > What is the procedure for further testing and possible inclusion with
> | > (the right) Octave (package)?
> |
> | Ideally, you'd produce a Mercurial changeset:
> |
> |     
> http://www.gnu.org/software/octave/doc/interpreter/Basics-of-Generating-a-Changeset.html
> |     http://jordi.inversethought.com/blog/how-to-write-a-patch-for-octave/
> |
> | The file you want to patch is scripts/audio/wavwrite.m inside the
> | Octave repository.
> |
> | At a first glance, your fix looks correct. You will need to produce a
> | commit message for it in this style:
> |
> |     http://wiki.octave.org/Commit_message_guidelines
> |
> | A few stylistic issues:
> |
> |    * You changed the function's name to wavwritex. I suppose this was
> |      for testing purposes. Please undo this.
> |
> |    * You use % and plain ends. This is Matlab-style. In Octave we use
> |      ## for comments and endfor, endif, endwhile, etc.
> |
> |    * In order to distinguish function calls from indexing operations,
> |      we put a space between the function name and the opening bracket
> |      in function calls, e.g.
> |
> |          a_function_call (x, y, z);
> |          an_indexing_op(idx);
> |
> |    * We don't do one-line ifs. Nor ifs without brackets around the
> |      condition. Instead of
> |
> |          if foo; bar (); endif
> |
> |      do
> |
> |          if (foo)
> |            bar ();
> |          endif
> |
> |    * Spaces around + and -. No spaces around * and /, e.g.
> |
> |          x + y*z - w/u
> |
> |      not
> |
> |          x+y*z-w/u
> |
> | If you can address these style issues and produce the hg changeset, we
> | can more easily incorporate your patch into Octave. If it's too
> | onerous for you to follow Octave coding style and to use hg, you can
> | just dump your proposed fix in the patch tracker[1], but bear in mind
> | that *someone* has to do the commit message, style fixes, etc. If you
> | don't do it, you're just passing that work on to someone else. I may
> | do it, but since I don't care too much about this function, I may not
> | get around to it as urgently as you might.
> |
> | Thanks for wanting to contribute,
>
> And more important than these style issues, it looks like your change
> uses a for loop and so will call fwrite 3*length(yi) times.  I expect
> that will be very slow.  Is there some reason that this operation
> can't be vectorized so that you build one large array using vector
> operations and then write that to the file in a single call to fwrite?
> I'm certain that will be much faster than the loop.
>
> jwe

wavwrite.m
Description: Binary data