More:

MP4::Info does not parse AACplus files properly for bitrate, etc.

Any way to support seeking?

Can faad2 support seeking on the command line (I have not looked).

When you say "MP4::Info does not parse AACplus files properly for bitrate", do you mean that the code is in error, or that it is missing. The CPAN version says that it should return this info.

It doesn't appear to support seeking.

I haven't looked into it yet, but I encoded a file using the 3GPP AACplus encoder (found a Mac binary here: http://blog.massanti.com/2007/09/30/he-aac-plus-encoder-mac-universal-binary/) and it plays fine but the scanner doesn't pick up all the info about it.

When you say AACPlus, do you mean HE-AAC or iTunes Plus?

HE-AAC == AACplus

AAC is a mess of different names and standards.

Ah, thanks.  Yeah, it's a mess, especially with Apple making up their own names.

Do we have examples some where of the files in question?

I'm hoping bug 3174 will be addressed which covers creating a discrete AAC file type before this is fully implemented.

Yeah we need to think about how to do that.  I replaced the MOV string with just "AAC".  Does anyone even have actual Quicktime mov files?

Another thought we had was to possibly compile mplayer with only the faad decoder and no other codecs.  That should get us seeking and remote stream support.

CC bpa in case you might have some input.

I'm assuming you now have a patent license?

I think you are better off with a patched faad2 this would use pipelines for remote streams but need Bryans view on whether it would handle all.

The problem with changes to SC to handle killing faad is that I've never found one which handles all cases - i.e. the transcode on its own and the transcode called from a shell with a pipe to another transcoder.  Its much better if they handle sigpipe themselves.

Mplayer would also need such patching.

Yes we have obtained a license.

I'm happy to stay with faad if we can solve these issues easier there.

Seeking needs some extensions to frontend/main.c - I've not looked enough at the libary but assume the decode routine will sync to whereever it is asked to read in a file.  If so would be minimal.

Some of the potential issues with using mplayer included with SC.
* it is a bit of a heavyweight (i.e. memory usage) - it is a single executable
with all libraries linked which means it can be quite big.  I'm not sure if it
would be small even if you didn't include a lot of codecs as I think many of
the "transport" protocols will be included.  

* standard mplayer needs to be patched to handle stdin for input and redirect
stdout messages to stderr.  

* mplayer links into ncurses somewhere which needs a TERMINFO setting and a
termcap database - this has caused issues for some NAS users. This might need
to be tracked down and the occasional message to stdout patched.

* Even current mplayer doesn't shutdown reliably on SIGPIPE - I have a recent
version of mplayer for SuSE and mplayer doesn't shutdown when SC closes the
stream pipes whereas it shuts down OK on Ubuntu. 

I think it would be easier to maintain faad2.  

However besides lack of seeking - it could have small issues with AAC/AACplus
streams rather than files.  When working out SC support of AACplus using
mplayer - I had to get a patch added to the faad2 libraries in mplayer to deal
with handling streams:
* getting "synced" with a new stream (i.e. trying to find a place in the input
stream to start decoding)
* handling "no more data" in the middle of decoding frames. 

So it be worthwhile looking at the mplayer version of the faad libraries and
checking if there any useful mods.

OK, let's see how much we can do with faad for now.

I think so far we need patches for:

Reading from stdin
Seeking to a byte offset
Frame sync when seeking/streaming

Since you've done some of that already for libfaad in mplayer, would you be able to see if any of those patches will port back easily to the current version?

OK - I'll look into what libfaad changes have been made for mplayer.

I have started looking at mplayer libfaad and after briefly looking at a few files I found that mplayer is using libfaad pre-2.5 with some patches where as libfaad is currently 2.6.1 . 

At first glance there also seems to be a few small coding errors which are fixed libfaad but not in mplayer.  

Looking back at mplayer mailing list it looks like changes to libfaad license meant that mplayer couldn't/wouldn't accept new license terms and so have stayed with a base version of libfaad from around 2004/5 and patching.

So using faad2 would seem to be the better starting point.

Note one side effect of splitting aac from mov is that a full rescan is going to be required for everyone with AAC files.

(In reply to comment #17)
> Note one side effect of splitting aac from mov is that a full rescan is going
> to be required for everyone with AAC files.

Why is that?  Is there a way we could get around it?  What would be a good solution?  Having to do a full rescan every time does not sound like an acceptable side effect to me :(

Hmm, maybe during upgrade we could just change the content-type of all mov files in the database to aac?

.mov files can have audio of any codec type in them.  if it's a .mov file, we should let quicktime do the decoding

Anyone have any updates on the changes we need for faad?  We want to get this into 7.3.3.

I've not been looking at this - I was hoping Bryan was... 

I note that there is now a new release of faad2 which includes my patch for ending when the output pipe is broken.

I did look into the mplayer changes make to libfaad.  I hadn't looked what changes were need to faad2 to achieve the same functionality.

MPlayer uses libfaad unmodifed and has a wrapper layer which deals with the problem issues such as input unexpected closing, signals and synchronisation.  The patch I supplied went into this wrapper layer.

How do you want to proceed ?

I've updated the builds on 7.4 to faad 2.7.

I've backported this to 7.3.3 in change .  Currently a full rescan is required in order to use faad instead of mov123.  Should we automatically convert the content-type of files in the database?

Also at minimum, we need to support AACplus radio streams.  Seeking is optional but nice to have.

Sorry, that's change 25053.

I'll see if I can get AACplus streams to work with faad.

We also need to sort out why faad2 won't write WAV format to stdout. Forcing an assumption of 44100/2/16 is a bad idea.

Wav headers are bad for streaming radio stations - it limits the streaming to 3 hours and something at 44.1  [we've moved Alien back to raw pcm for this reason]

Not sure there is an easy way round this though.

That is a pity. Presumably this is either a flac limitation, or maybe mplayer just puts a bogus 3-hour length in the WAV header. I guess there could be a similar but different issue with faad.

I've got a preliminary version of FAAD which will work for streaming AACplus and AAC to a RAW PCM stream but I have difficulty with the convert.conf lines handling   AAC/AACplus treams from stdin and files from $FILE$

FAAD write a dummy WAV header when initial decode starts and then when decode is finished, seeks to start of output file and fills in the header details.  Seek fails on streamed output (i.e. stdout to a pipe)  and so will fail when doing the final update of the header.  FAAD could be modified not to write the updated header if the final seek fails. 

An alternative would be to modify faad frontend to call a resample library to ensure faad output is always to request format.

(In reply to comment #31)
> I've got a preliminary version of FAAD which will work for streaming AACplus
> and AAC to a RAW PCM stream but I have difficulty with the convert.conf lines
> handling   AAC/AACplus treams from stdin and files from $FILE$

I think that what you are saying is that you want it to read stdin for AACplus streams and read the file directly for local files. I think you are right that the framework does not allow for that at present (I guess it should). Is that actually necessary? I seem to remember faad having problems reading AAC files via stdin but I don't know why.

> FAAD write a dummy WAV header when initial decode starts and then when decode
> is finished, seeks to start of output file and fills in the header details. 
> Seek fails on streamed output (i.e. stdout to a pipe)  and so will fail when
> doing the final update of the header.  FAAD could be modified not to write the
> updated header if the final seek fails. 
> 
> An alternative would be to modify faad frontend to call a resample library to
> ensure faad output is always to request format.

It would be nicer if faad could output a sufficiently-correct WAV header at the beginning, so that it does not need to seek back, then if seeking fails (should not even try for stdout) it is no big deal.

To support AAC and AACplus radio streams I have made mods to allow stdin as an input.  

AFAICT Standard FAAD scans a file twice during conversion - once to get header and statistics and duration and 2nd time to convert.  To do this it does some seeks within the file and seeking is not supported on stdin so it fails.

MPEG4 headers are complicated and usually media data is at the end,  the actual media can be embedded in a MPeg4 header and so again FAAD seems to use seeks within the file rather than try to process linearly.

The above limitations are not in libfaad but in the FAAD support routines mp4ff and frontend.  MPlayer using the same libfaad can process a AAC or MPEG4 stdin streams and files linearly with no seeks because it buffers a lot and emulates seeks in the buffer (libfaad provides read & seek callbacks).  

To avoid rewriting a lot of faad,  I am going to try replacing aac type by two types say aacf for files (i.e. local files) and aacs for streams (i.e. http with mime audio/aac & audio/aacp) to see if my current mods are sufficient. However I think this approach won't support MPEG4 podcasts which are growing in popularity but it would cover the bulk of AAC/AAC+/MPEG4 needs.

Regarding support of AAC/AACplus stream, the following works for a limited set of files and streams I have tested.  Is it acceptable and be tested further or should more mods (or even create a new "frontend") be made to faad ?   

With the mods to faad to accept AAC and AACplus streams on stdin but files cannot use stdin (i.e. must have file name).

types.conf file has the aac type changed to 
aacf    m4a,mp4,m4b,aac    audio/m4a,audio/x-m4a           audio
aacs    -                  audio/aac,audio/aacp            audio

convert.conf lines for AAC->Flac
aacf flc * *
	# F
	[faad] -q -w -f 2 -b 1 -s 44100 $FILE$ | [flac] -cs --totally-silent --compression-level-0 --endian little --sign signed --channels 2 --bps 16 --sample-rate 44100 -

aacs flc * *
	# I
	[faad] -q -w -f 2 -b 1 -s 44100 $FILE$ | [flac] -cs --totally-silent --compression-level-0 --endian little --sign signed --channels 2 --bps 16 --sample-rate 44100 -

This approach will also maximise support across platforms,  users can easily replace the aacs entries to use other apps such as mplayer if the the user cannot build their own version of the special FAAD.

The separate issue about streams or file whose formats are not 2 channel 44.1kHz.  Anything can be put in a WAV header and the seek/rewrite can be suppressed if output is a stream but that will not solve the problem of lame or Flac reading a WAV header and stopping when the header count is reached.  The only solution would be special version of flac/lame which does not seem practical as it would complicate support on many platforms.

(In reply to comment #34)

> The separate issue about streams or file whose formats are not 2 channel
> 44.1kHz.  Anything can be put in a WAV header and the seek/rewrite can be
> suppressed if output is a stream but that will not solve the problem of lame or
> Flac reading a WAV header and stopping when the header count is reached.  The
> only solution would be special version of flac/lame which does not seem
> practical as it would complicate support on many platforms.

How big a value would the WAV header hold? I mean, could it put a place-holder of 10 years there, and fix up the right value if needed and possible?

I think that using two different file types makes some sense, as they are indeed being handled differently, even if by the same program in this case.

WAV header has a count file for audio data. It is 32 bit counter which is defined as follows
 NumSamples * NumChannels * BitsPerSample/8

So 0xFFFFFFFE (I think) is approx  3h 20mins of 2 channel, 44.1kHz audio.

Bryan: can you post your patches so I can build binaries with them, and we can start getting these changes implemented?

I think the 3 hours 20 minute issue counts out using a wav header for the streaming case - in this case it is unfortunate that we need to assume a bit rate though when transcoding.  Looking at aiff I think this also includes a 32 bit length field which would probably result in the same issue.

This is a problem as 48kHz sampled aac streams will be common (I suspect this will include the BBC's new aac streams should they become available outside of flash).  Can we do anything in faad to get the sample rate to the second process in the pipeline, or scan it in advance and add it to the variables which are substibuted for the convert command?

Created attachment 4858 [details]
Patch file for FAAD2 2.7 frontend/main.c 

Patch file attached for frontend/main.c of faad2 2.7.  The patch enables input on stdin for AAC/AAC+ streams. I have only built and tested on Linux.

FAAD cannot do anything in advance for streams so I think the only reasonable solution would be a FAAD to pipe its output through a child process such as lame /or Flac. The actual command template to be used could be a command line parameter. The other choice would be for FAAD be modified to convert/resample input to make sure it always outputs a standard format (e.g. 2 channel 44.1kHz).

I tend to think we want to avoid resampling in faad.  However piping the output into a subprocess and then out again would probably require implementing more stuff in faad and would result on doing something similar to socketwrapper...

I think I have found the source of the 3 hour 20 minute limit - this is 4GB worth of samples at 44.1.  The flac encode front end has a 4G limit in flac__encode_wav which does not exist in flac__encode_raw.

I wonder if we should patch flac to avoid this.  I think the culprit is data_bytes in flac_encode_wav which is an unsigned int.  There appears to be a flac command line switch to ignore the length in the wav header (--ignore-chunk-sizes) but when you do this it just bumps data_bytes upto 4GB.  We could possibly just patch this to avoid using this counter if the --ignore-chunk-sizes switch is set?  

Can we get the QA team to run some tests to prove the 3 hours 20 minutes limit and potentiall provide two versions of flac to test.  I'm happy to propose a patch, but don't have time to test for hours!

I think I understand the wav file length issue now, at least for flac:

1) with Alien we use mplayer.  This sets the wave length field to 0x7ffff000 if a wav header is written.  [mplayer/libao/ao_pcm.c in init]  It will go back and update the header when it closes the file, but this is irrelavant to us.

0x7ffff000 - is close to 2G and equates to ~ 3 hours 20 mins of samples

2) with Faad, the wav header is written based on total_samples and if this is too high it is written as MAXWAVESIZE which is 4294967040LU.  [The wav header is writen in frontend/audio.c in write_wav_header.]

4294967040 bytes is 6 hours 45 mins at 44100

3) flac will read the wav header and use it to set data_bytes in src/flac/encode.c flac__encode_wav.  If --ignore-chunk-sizes is set on the command line then it is ignored and data_bytes is set to 4G.

So if we use --ignore-chunk-sizes we can move this problem to the 6 hour mark.  However I think there is a trivial patch to flac to avoid it completely when --ignore-chunk-sizes is set and I think this is in the spirit of how the flac front end is written so it should be submitted upstream:

--- /home/adrian/jive/fab4/squeezeplay/src/flac-1.2.1/src/flac/encode.c 2008-12-23 11:40:28.000000000 +0000
+++ encode.c    2009-03-01 00:22:16.000000000 +0000
@@ -1061,7 +1061,10 @@
                                                        print_error_with_state(&encoder_session, "ERROR during encoding");
                                                        return EncoderSession_finish_error(&encoder_session);
                                                }
-                                               data_bytes -= bytes_read;
+
+                                               if (!options.common.ignore_chunk_sizes) {
+                                                       data_bytes -= bytes_read;
+                                               }
                                        }

So I conclude that we should use the wav header as this allows us to get the sampling rate into flac.  However we need to use --ignore-chunk-sizes for flac and ideally also patch flac to avoid remote streams stopping at the 6 hour mark.

Should I build binaries using bpa's latest patch, or wait for more changes?

Triode, that looks like he right solution. I have added bug 11223 for a patched flac.

I have Bryan's patched version running with AAC streams - I suggest we go with this.  The convert.conf entries should use wave headers, same type.conf entries as bryan has.

aacf flc * *
    # F
    [faad] -q -w -f 1 $FILE$ | [flac] -cs --totally-silent --compression-level-0 --ignore-chunk-sizes -

aacs flc * *
    # I
    [faad] -q -w -f 1 $FILE$ | [flac] -cs --totally-silent --compression-level-0 --ignore-chunk-sizes -

aacf mp3 * *
    # F
    [faad] -q -w -f 1 $FILE$ | [flac] --silent -q $QUALITY$ -b $BITRATE - -

aacs mp3 * *
    # I
    [faad] -q -w -f 1 $FILE$ | [lame] --silent -q $QUALITY$ -b $BITRATE - -

Once flac is patched as per the other bug this should run for ever, but without it, it should run for 6 hours.  Lame does not look to use the data length in the wav header to decide when to stop encoding so I think this will be ok as is for lame.

I've checked in new binaries with bpa's patch (change 25247).  Please go ahead and make the necessary types changes.

Been testing with my build of bpa's patch and the patched flac and its still playing 7 hours later on an aac radio stream.

Bryan - could you check change 25252 - I think its ok to use the same convert commands for streaming and local files.  These seem to work for me.

Also can you put it in 7.3 trunk?  faad is for 7.3.3.

All my test AACplus streams play OK.  I cannot find a stream with 48kHz to test.

My test m4a files do not play. I'll do some more checking but this was the problem  that was solved by using two types.

My test m4a files play OK if I just change the IF to F in the convert.conf file.

I think this relates to files with MPEG4 header - faad seeks up and down the file to decode an MPEG4 header. When the "IF" is used then faad input is via stdin "-" and faad cannot seek on stdin. When "F" is used then the file name is provided to faad and it can seek.

I should have thought of this earlier. I wonder if sox has a time limit when reading wav from stdin. If not then we could use sox instead of flac, and also get downsampling capability thrown in.

Hum - I tested with aac rather than an m4a file.

I will test again.  However I think the transcoding framework is broken if it is passing local files to the transcoder binary over a pipe - its making SC doing much more work that it needs to do, so I think the correct answer is to make it so SC passes the filename not a file handle.

There are good reasons why it prefers stdin to the filename if both are available. I think that there may be some improvement possible there but not in the 7.3.3 timeframe.

Surely we don't want to create a pipeline or a socketwrapper process unless we absolutely need to - we don't need to do this for local files?  So why can't we just test if the file is local and if so not do this?

Otherwise I've confirmed faad will require another set of convert entries for m4a files (aac files work with the current settings, but we don't want to be spawning extra processes and socketwrapper on windows so we should fix for this too)

I think it's possible to handle most file with mpeg4 headers using stdin but it requires rewriting the faad mpeg4 header handling. 

I wonder whether this mpeg4 header issues is one of the causes behind the forums posts that aac files are not playing with 7.3.3.

I doubt it. You only get stdin if the transcoder config includes 'I'. Most of them are configured with just 'F'. The difficulties can only arise with ones that need to support both, such as Ogg, for both local files and for streams (although with Ogg one wants stdin anyway because that way SC can do seeking.)

I think this means we need 2 sets of convert commands for the moment - I've made them aac and mp4 as there are some places on the UI (e.g. web interface) where they show up without being localised.

Added in svn 25272

Assuming this works it needs porting to 7.3

I've tested and it plays AAC+ streams and m4a with ,mpeg4 headers OK. I have no files with aac headers to test.

Can and should m4a with MPEG4 header play gapless ?

I have some gapless WAV test files which play gapless when converted to Flac but do not play gapless when converted and played as M4a/mpeg4 header files.

I am not sure if the problem the conversion, inability of m4a to play gapless or a bug in faad/flac chain.

Just wanted to note that I am unable to play 48 kHz AAC sourced from iTunes.  File attached to bug 8562.

Steven - what hardware are you having problems with and what transcoding are you using.  That 48 kHz file seems to work for me on squeezeplay and Boom.

Really?  I was using an SB3 at the time.  Will investigate further.

Change 25403 adds support for recognising that MP4 (M4a, etc.) files may contain Apple Lossless encoding.

I tried using sox instead of flac for FLAC encoding but sox gave up immediately. I guess it did not like the WAV header in some way. I have not investigated further yet.

I think the wav header will have a length of 0 to start with and faad will go back and update it for a real file by seeking to the start and overwriting it (mplayer does something similar).

By using the command line option with flac we can force it to ignore the length field in the header.  Its probably worth reading the sox source to see what it does, but with the change to flac I think it and lame are ok.  The only downside is no resampling - not sure if this is a major issue for the target platforms?

AAC+  streams do not work on Windows in recent 7.4 builds. A few users have reported it.  I'm checking it out as I didn't test the faad mods on Windows.

I found the problem in faad.  On Windows socketwrapper is used and it uses named pipes.  In Windows the fseek test on input works for named pipes so faad thought input was from a file.  

The simple fix is if input is "-" and on Windows then assume a stream but need confirmation that "aac" type is NEVER used with files. 

Why is socketwrapper being used when streaming faad/flac when it is not used when streaming mms streams using wmadec/flac ?

I've done some testing with AAC and M4A file with the patched FAAD, it seems to work OK.  I'll post the updated patch later.

Because streaming is now assumed when stdin is used - if ffw/rew is ever implemented, I think another "type" will be needed to separate handling files & streams.

> Why is socketwrapper being used when streaming faad/flac when it is not used
> when streaming mms streams using wmadec/flac ?

Socketwrapper is always used when a pipeline is created.  From the previous discussion a pipeline is created whenever there is an "I" on the convert command capabilities.  

I am not sure about the named pipe bit - we ought to be able to stream via stdin with faad and socketwrapper?  Socketwrapper was originally written to allow streaming via stdin, I added the named pipe bit purely for Alien as I did not want to modify mplayer.  So I think we should be able to make it stream via stdin and socketwrapper...

Created attachment 4917 [details]
Update patch file to handles stream.  Apply to original main.c

I didn't check the sockertwrapper code but in faad - there is an fseek test to determine whether input is a stream or just a redirected file. On Linux fseek on a pipe fails. On Windows the same fseek is successful on the faad input stream as set up by socketwrapper.

Attached is patch for main.c which differs against faad 2.7 original.  The additional fix makes the Win32 version of faad always fail fseek on stdin "-" input.

Separately a user with OSX and PowerPC processor has reported that AAC+ streams return an error "Problem: Unable to play file"

This patch replaces the previous main.c patch right?

Yes - it replaced the first patch I posted.

The new patch just adds a few more lines which are inside a "#ifdef WIN32" and so only affect the WIN32 build.

Hmm, the 2 patch files are identical.

Created attachment 4919 [details]
updated patch to original main.c

This should be the correct main.diff with the new changes.  I uploaded previous diff file from the wrong directory

Updated builds checked in, change 25559.

We are now planning to make a 7.3.3 release.  Please review your bugs (all marked open against 7.3.3) to see if they can be fixed in the next few weeks, or if they should be retargeted for 7.4 or future.

Thanks!

Since there's now a planned 7.3.3 release, bugs which won't make the cut-off are being moved to the next target out.  If you feel that this bug needs to be addressed more (or less) urgently than the 7.4 release, please cc chris@slimdevices.com and leave a comment in the bug to that effect so we can review it.

Thanks.

For some reason Bugzilla did not change the target when I did this yesterday.  Or maybe it was me.  In either case, I'm trying it again.

This is actually still 7.3.3.  Should we consider this one done or is there more to do?

Other than the bug report related to macs isn't this done?

The mac issue is the only problem I know about.  The user with the Mac issue has a PPC with OSX 10.4.x which I think is a older system. I assume that faad2 has been tested on Intel osx 10.5.x and so the problem may be a build issue and limited to older systems but this needs confirmation. 

If it is a build issue then other special builds (e.g. flac) in Bin should be tested on the same problematic system.

Matt: Are we building the faad2 for both PPC and x86 for the Mac?

Yes faad is a Universal Binary for 10.3+.  QA: please test on PPC Macs.

New faad2 binary tested on PowerPC based Mac with 10.3.9, 10.4.11 and 10.5.6 successfully.  Thanks Andy!

Marking as verified as per Stevens Comment #84.

This bug has been fixed in the 7.3.3 release version of SqueezeCenter!

If you haven't already. please download the new version from http://www.logitechsqueezebox.com/support/download-squeezecenter.html 

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.